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

[Devel] [PATCH hci-8.1 v6 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 v6:
- Patch 1, 5: build the test for x86-64 only, decided by the compiler
  target in the Makefile since ARCH says "x86" for 32-bit as well; the
  prefixed CPUID goes back to a plain "=b" asm, the 32-bit xchg around
  ebx of v5 truncated the callee-saved rbx on x86-64 (Sashiko).
- Patch 1: say in the comment and the message that the context switch
  the parent waits for need not be the stop itself: any switch after
  the count was taken arms faulting on the way back in (Sashiko).
- Link to v5: http://lore.virtuozzo.com/20261007-b4-prep-khorenko-vstor-148656-cpuid-v5-0-652c5c9e56da@virtuozzo.com

Changes in v5:
- Patch 1: the stop is reported to the parent before the child leaves
  the CPU, so the child hands over its voluntary context switch count
  and the parent waits for it to grow before SIGCONT; a SIGCONT in that
  window would have let the child run on without a switch (Sashiko).
- Patch 1, 5: CPUID through __cpuid_count() and an xchg around ebx for
  the prefixed form, so the 32-bit PIC build is no concern and the
  Makefile gate can keep ARCH=x86 (Sashiko).
- Patch 1: a failed table restore in the teardown fails the test
  instead of only logging; skip without root; log when the teardown
  cannot mount cgroup2; wider buffer for /proc/<pid>/status; the
  header says one instance at a time (independent review).
- Patch 5: early-clobber on the ebx substitute register in the
  prefixed CPUID asm, as <cpuid.h> has it (independent review).
- Link to v4: http://lore.virtuozzo.com/20261007-b4-prep-khorenko-vstor-148656-cpuid-v4-0-bf16c29bea24@virtuozzo.com

Changes in v4:
- Patch 1: a child that exits before stopping is reaped by the ready
  wait, so its pid is cleared there and the teardown cannot kill a
  reused pid (Sashiko).
- Patch 1: spawn leaves the VE cgroup before any ASSERT can fire, so a
  failed fork() no longer leaves the cgroup behind; a child killed
  before it stops is logged; the teardown is described as running in
  whichever process survives the ASSERT, the harness does not run it
  in the parent on that path (independent review).
- Patch 1: use write_file_at() instead of a private copy, renumber the
  child exit codes, say "at least" in the too-large table message.
- Patch 1: record the child pid right after fork(), so a child left
  stopped by an ASSERT between fork() and the cgroup move is still
  killed; say that a harness timeout leaves the test table in place
  and that the second case proves nothing without the first
  (independent review).
- Patch 1: name the fix the tests detect, describe the host-side and
  fallback checks as the one masked bit they are, log errno when the
  table read or the cgroup2 mount fails, rename the two table buffers
  apart, -Wextra clean (independent review).
- Link to v3: http://lore.virtuozzo.com/20261006-b4-prep-khorenko-vstor-148656-cpuid-v3-0-16141b8ca07d@virtuozzo.com

Changes in v3:
- Patch 1: a saved table the kernel cannot take back in one write (a
  page or more as printed) fails the setup instead of being silently
  left behind by a failed restore; writes replace the table, so there
  is no way to restore it in pieces (Sashiko).
- Patch 1: the child pid lives in the fixture and the parent-side
  teardown kills a child that a failed ASSERT left stopped in the VE
  cgroup, otherwise destroy_ve() fails with EBUSY (Sashiko).
- Patch 1: the Makefile keeps ARCH untouched and decides on a private
  variable, lib.mk picks the clang target by ARCH being empty (Sashiko).
- Link to v2: http://lore.virtuozzo.com/20261006-b4-prep-khorenko-vstor-148656-cpuid-v2-0-f6d1192c7d60@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; 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                |   6 +
 .../testing/selftests/ve/ve_cpuid_override_test.c  | 357 +++++++++++++++++++++
 7 files changed, 390 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 v6 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 must keep the hardware
bit. A second test flushes the table after the task was armed and
expects the hardware bit back, since faulting stays on for the task
and the emulation has to fall back to the real CPUID. On its own that
test would also pass on a kernel that never arms faulting; the first
one proves the arming.

Faulting is switched on for a task at its next context switch, not
when the flag is set, so the child stops itself with SIGSTOP after
entering the VE and the parent resumes it before the CPUID. The stop
is reported to the parent before the child leaves the CPU, and a
SIGCONT in that window would let it run on without a switch, so the
child hands over its voluntary context switch count first and the
parent waits for it to grow before the SIGCONT. Any switch after that
count was taken arms faulting on the way back in, so it does not
matter whether the stop or an earlier sleep is the one counted. For
the second test the table is flushed while the child stands still.

Without commit 07338fbae5a3 ("x86/traps: emulate CPUID faults for user
tasks in exc_general_protection()") both children die with SIGSEGV.
The tests skip on CPUs without CPUID faulting and restore the previous
table on exit; a table the kernel cannot take back in one write fails
the setup instead. The harness may skip FIXTURE_TEARDOWN once an ASSERT
has fired, so the cleanup is a FIXTURE_TEARDOWN_PARENT, which runs
exactly once whether or not one did; it also kills a child that such
an ASSERT left stopped in the VE cgroup. The test is built on x86-64 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                |   6 +
 .../testing/selftests/ve/ve_cpuid_override_test.c  | 298 +++++++++++++++++++++
 3 files changed, 305 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..3f494a49270e5 100644
--- a/tools/testing/selftests/ve/Makefile
+++ b/tools/testing/selftests/ve/Makefile
@@ -6,4 +6,10 @@ TEST_GEN_PROGS += ve_ns_owner_test
 TEST_GEN_PROGS += ve_perms_test
 TEST_GEN_PROGS += ve_mount_accounting_test
 
+# CPUID faulting is x86-64 only. ARCH says "x86" for 32-bit too and lib.mk
+# picks the clang target by its emptiness, so decide by the compiler.
+ifneq ($(filter x86_64-%,$(shell $(CC) -dumpmachine 2>/dev/null)),)
+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..a56e0c7213fb9
--- /dev/null
+++ b/tools/testing/selftests/ve/ve_cpuid_override_test.c
@@ -0,0 +1,298 @@
+// 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.
+ *
+ * The table is global: run this on a node without live Containers and one
+ * instance at a time, a task entering a VE while a case runs is armed for
+ * life. A harness timeout kills the test without its teardown and leaves
+ * the test table in place and the VE cgroup behind.
+ */
+#define _GNU_SOURCE
+#include <cpuid.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdio.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_LINE_LEN		128
+/* The kernel takes a table back only in one write below a page. */
+#define CPUID_WRITE_MAX		4096
+#define CPUID_SAVED_MAX		(CPUID_WRITE_MAX + 1)
+#define SWITCH_WAIT_ROUNDS	100000
+
+struct cpuid_regs {
+	unsigned int eax, ebx, ecx, edx;
+};
+
+static void do_cpuid(unsigned int leaf, struct cpuid_regs *r)
+{
+	__cpuid_count(leaf, 0, r->eax, r->ebx, r->ecx, r->edx);
+}
+
+/* Voluntary context switches of a task, from its /proc status file. */
+static int read_nvcsw(const char *status, unsigned long *nvcsw)
+{
+	char buf[8192], *p;
+
+	if (read_file_at(AT_FDCWD, status, buf, sizeof(buf)) <= 0)
+		return -1;
+	p = strstr(buf, "\nvoluntary_ctxt_switches:");
+	if (!p || sscanf(p, "\nvoluntary_ctxt_switches: %lu", nvcsw) != 1)
+		return -1;
+	return 0;
+}
+
+struct child_ctx {
+	int result_pipe[2];
+};
+
+/*
+ * 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. It tells the parent its context switch count first, so
+ * that the parent can wait for the switch out.
+ */
+static int cpuid_child(struct child_ctx *ctx)
+{
+	struct cpuid_regs r;
+	unsigned long nvcsw;
+
+	close(ctx->result_pipe[0]);
+
+	if (unshare(CLONE_NEWVE) < 0)
+		return 21;
+	if (read_nvcsw("/proc/self/status", &nvcsw))
+		return 22;
+	if (write(ctx->result_pipe[1], &nvcsw, sizeof(nvcsw)) != sizeof(nvcsw))
+		return 23;
+	if (raise(SIGSTOP))
+		return 24;
+
+	do_cpuid(CPUID_LEAF, &r);
+
+	if (write(ctx->result_pipe[1], &r, sizeof(r)) != sizeof(r))
+		return 25;
+	return 0;
+}
+
+FIXTURE(ve_cpuid_override)
+{
+	int cgv2_fd;
+	int ctid;
+	struct cpuid_regs host;
+	unsigned int bit;
+	pid_t child;
+	char table[CPUID_LINE_LEN];
+	char saved[CPUID_SAVED_MAX];
+};
+
+FIXTURE_SETUP(ve_cpuid_override)
+{
+	int fd, len;
+
+	if (geteuid())
+		SKIP(return, "needs root");
+	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) TH_LOG("%s: %s", CPUID_OVERRIDE_PATH, strerror(errno));
+	ASSERT_LT(len, CPUID_WRITE_MAX)
+		TH_LOG("a table of at least %d bytes cannot be restored", len);
+
+	/* 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, 0u);
+
+	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) TH_LOG("cgroup2 mount: %s", strerror(errno));
+	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 after the test body in whichever process the harness picks, the test
+ * process itself after a failed ASSERT, so the table is restored whether or
+ * not one fired. The setup fd may be gone by then, hence the fresh mount.
+ */
+FIXTURE_TEARDOWN_PARENT(ve_cpuid_override)
+{
+	int cgv2_fd;
+
+	EXPECT_EQ(write_file_at(AT_FDCWD, CPUID_OVERRIDE_PATH,
+				self->saved[0] ? self->saved : "\n"), 0)
+		TH_LOG("could not restore %s: %s", CPUID_OVERRIDE_PATH,
+		       strerror(errno));
+
+	/* A child left stopped by a failed ASSERT keeps the VE cgroup busy. */
+	if (self->child > 0)
+		kill(self->child, SIGKILL);
+
+	cgv2_fd = mount_cg2_fd();
+	if (cgv2_fd < 0) {
+		TH_LOG("cgroup2 mount: %s, VE cgroup %d left behind",
+		       strerror(errno), self->ctid);
+		return;
+	}
+	destroy_ve(cgv2_fd, self->ctid);
+	close(cgv2_fd);
+}
+
+/*
+ * Stopped is reported to the parent before the child leaves the CPU, so a
+ * SIGCONT sent right away could keep it running without a switch. Wait
+ * until its voluntary context switch count has moved on: any switch after
+ * the count was taken, which is after the flag was set, arms faulting on
+ * the way back in, whether it is the stop or an earlier sleep.
+ */
+static void wait_switched_out(struct __test_metadata *_metadata, pid_t pid,
+			      unsigned long nvcsw_before)
+{
+	char status[64];
+	unsigned long nvcsw;
+	int i;
+
+	snprintf(status, sizeof(status), "/proc/%d/status", pid);
+	for (i = 0; i < SWITCH_WAIT_ROUNDS; i++) {
+		ASSERT_EQ(read_nvcsw(status, &nvcsw), 0);
+		if (nvcsw > nvcsw_before)
+			return;
+		sched_yield();
+	}
+	ASSERT_GT(nvcsw, nvcsw_before);
+}
+
+/* Fork the child into the VE and return once it is off the CPU. */
+static pid_t spawn_cpuid_child(struct __test_metadata *_metadata,
+			       FIXTURE_DATA(ve_cpuid_override) * self,
+			       struct child_ctx *ctx)
+{
+	unsigned long nvcsw;
+	int status;
+	pid_t pid;
+
+	ASSERT_EQ(pipe(ctx->result_pipe), 0);
+	ASSERT_GE(enter_cgroup(self->cgv2_fd, self->ctid), 0);
+
+	pid = fork();
+	if (pid == 0)
+		_exit(cpuid_child(ctx));
+	if (pid > 0)
+		self->child = pid;
+
+	/* Leave the VE cgroup before any ASSERT, or destroy_ve() gets EBUSY. */
+	ASSERT_GE(enter_cgroup(self->cgv2_fd, 0), 0);
+	ASSERT_GE(pid, 0);
+	close(ctx->result_pipe[1]);
+
+	ASSERT_GE(waitpid(pid, &status, WUNTRACED), 0);
+	if (!WIFSTOPPED(status)) {
+		/* Reaped: the pid may be reused, keep it from teardown. */
+		self->child = 0;
+		if (WIFEXITED(status))
+			TH_LOG("child exited with %d", WEXITSTATUS(status));
+		if (WIFSIGNALED(status))
+			TH_LOG("child killed by signal %d", WTERMSIG(status));
+	}
+	ASSERT_TRUE(WIFSTOPPED(status));
+	ASSERT_EQ(read(ctx->result_pipe[0], &nvcsw, sizeof(nvcsw)),
+		  (int)sizeof(nvcsw));
+	wait_switched_out(_metadata, pid, nvcsw);
+	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,
+				FIXTURE_DATA(ve_cpuid_override) * self,
+				struct child_ctx *ctx, struct cpuid_regs *r)
+{
+	int status, len;
+
+	len = read(ctx->result_pipe[0], r, sizeof(*r));
+	ASSERT_GE(waitpid(self->child, &status, 0), 0);
+	self->child = 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, (int)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_file_at(AT_FDCWD, CPUID_OVERRIDE_PATH, self->table), 0);
+
+	pid = spawn_cpuid_child(_metadata, self, &ctx);
+	resume_cpuid_child(_metadata, pid);
+	collect_cpuid_child(_metadata, self, &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, 0u);
+}
+
+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_file_at(AT_FDCWD, CPUID_OVERRIDE_PATH, self->table), 0);
+
+	pid = spawn_cpuid_child(_metadata, self, &ctx);
+
+	ASSERT_EQ(write_file_at(AT_FDCWD, CPUID_OVERRIDE_PATH, "\n"), 0);
+	resume_cpuid_child(_metadata, pid);
+	collect_cpuid_child(_metadata, self, &ctx, &r);
+
+	EXPECT_NE(r.edx & self->bit, 0u);
+}
+
+TEST_HARNESS_MAIN

-- 
2.43.0

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

[Devel] [PATCH hci-8.1 v6 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 v6 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)
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 .../testing/selftests/ve/ve_cpuid_override_test.c  | 83 +++++++++++++++-------
 1 file changed, 57 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/ve/ve_cpuid_override_test.c b/tools/testing/selftests/ve/ve_cpuid_override_test.c
index a56e0c7213fb9..f6a6325e4b42d 100644
--- a/tools/testing/selftests/ve/ve_cpuid_override_test.c
+++ b/tools/testing/selftests/ve/ve_cpuid_override_test.c
@@ -32,6 +32,7 @@
 #define CPUID_OVERRIDE_PATH	"/proc/vz/cpuid_override"
 #define CPUID_LEAF		1
 #define CPUID_LINE_LEN		128
+#define NO_RESCHED_ROUNDS	20
 /* The kernel takes a table back only in one write below a page. */
 #define CPUID_WRITE_MAX		4096
 #define CPUID_SAVED_MAX		(CPUID_WRITE_MAX + 1)
@@ -61,15 +62,16 @@ static int read_nvcsw(const char *status, unsigned long *nvcsw)
 
 struct child_ctx {
 	int result_pipe[2];
+	int stop;
 };
 
 /*
  * 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. It tells the parent its context switch count first, so
- * that the parent can wait for the switch out.
+ * 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. It tells the parent its context switch count
+ * first, so that the parent can wait for the switch out.
  */
 static int cpuid_child(struct child_ctx *ctx)
 {
@@ -80,12 +82,15 @@ static int cpuid_child(struct child_ctx *ctx)
 
 	if (unshare(CLONE_NEWVE) < 0)
 		return 21;
-	if (read_nvcsw("/proc/self/status", &nvcsw))
-		return 22;
-	if (write(ctx->result_pipe[1], &nvcsw, sizeof(nvcsw)) != sizeof(nvcsw))
-		return 23;
-	if (raise(SIGSTOP))
-		return 24;
+	if (ctx->stop) {
+		if (read_nvcsw("/proc/self/status", &nvcsw))
+			return 22;
+		if (write(ctx->result_pipe[1], &nvcsw, sizeof(nvcsw)) !=
+		    sizeof(nvcsw))
+			return 23;
+		if (raise(SIGSTOP))
+			return 24;
+	}
 
 	do_cpuid(CPUID_LEAF, &r);
 
@@ -195,7 +200,7 @@ static void wait_switched_out(struct __test_metadata *_metadata, pid_t pid,
 	ASSERT_GT(nvcsw, nvcsw_before);
 }
 
-/* Fork the child into the VE and return once it is off the CPU. */
+/* Fork the child into the VE; with @stop return once it is off the CPU. */
 static pid_t spawn_cpuid_child(struct __test_metadata *_metadata,
 			       FIXTURE_DATA(ve_cpuid_override) * self,
 			       struct child_ctx *ctx)
@@ -218,19 +223,23 @@ static pid_t spawn_cpuid_child(struct __test_metadata *_metadata,
 	ASSERT_GE(pid, 0);
 	close(ctx->result_pipe[1]);
 
-	ASSERT_GE(waitpid(pid, &status, WUNTRACED), 0);
-	if (!WIFSTOPPED(status)) {
-		/* Reaped: the pid may be reused, keep it from teardown. */
-		self->child = 0;
-		if (WIFEXITED(status))
-			TH_LOG("child exited with %d", WEXITSTATUS(status));
-		if (WIFSIGNALED(status))
-			TH_LOG("child killed by signal %d", WTERMSIG(status));
+	if (ctx->stop) {
+		ASSERT_GE(waitpid(pid, &status, WUNTRACED), 0);
+		if (!WIFSTOPPED(status)) {
+			/* Reaped: the pid may be reused, keep it from teardown. */
+			self->child = 0;
+			if (WIFEXITED(status))
+				TH_LOG("child exited with %d",
+				       WEXITSTATUS(status));
+			if (WIFSIGNALED(status))
+				TH_LOG("child killed by signal %d",
+				       WTERMSIG(status));
+		}
+		ASSERT_TRUE(WIFSTOPPED(status));
+		ASSERT_EQ(read(ctx->result_pipe[0], &nvcsw, sizeof(nvcsw)),
+			  (int)sizeof(nvcsw));
+		wait_switched_out(_metadata, pid, nvcsw);
 	}
-	ASSERT_TRUE(WIFSTOPPED(status));
-	ASSERT_EQ(read(ctx->result_pipe[0], &nvcsw, sizeof(nvcsw)),
-		  (int)sizeof(nvcsw));
-	wait_switched_out(_metadata, pid, nvcsw);
 	return pid;
 }
 
@@ -259,7 +268,7 @@ static void collect_cpuid_child(struct __test_metadata *_metadata,
 
 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;
 
@@ -280,7 +289,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;
 
@@ -295,4 +304,26 @@ TEST_F(ve_cpuid_override, flushed_table_falls_back_to_hardware)
 	EXPECT_NE(r.edx & self->bit, 0u);
 }
 
+/*
+ * 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;
+	int i;
+
+	ASSERT_EQ(write_file_at(AT_FDCWD, CPUID_OVERRIDE_PATH, self->table), 0);
+
+	for (i = 0; i < NO_RESCHED_ROUNDS; i++) {
+		struct child_ctx ctx = { .stop = 0 };
+
+		spawn_cpuid_child(_metadata, self, &ctx);
+		collect_cpuid_child(_metadata, self, &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 v6 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 v6 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  | 30 +++++++++++++++++++++-
 1 file changed, 29 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 f6a6325e4b42d..b9f5e49d6c437 100644
--- a/tools/testing/selftests/ve/ve_cpuid_override_test.c
+++ b/tools/testing/selftests/ve/ve_cpuid_override_test.c
@@ -47,6 +47,15 @@ static void do_cpuid(unsigned int leaf, struct cpuid_regs *r)
 	__cpuid_count(leaf, 0, r->eax, r->ebx, r->ecx, r->edx);
 }
 
+/* CPUID with an operand-size prefix, which the CPU accepts and ignores. */
+static void do_cpuid_prefixed(unsigned int leaf, struct cpuid_regs *r)
+{
+	/* An x86-64 only build, see the Makefile: ebx is not the PIC base. */
+	asm volatile(".byte 0x66, 0x0f, 0xa2"
+		     : "=a" (r->eax), "=b" (r->ebx), "=c" (r->ecx), "=d" (r->edx)
+		     : "a" (leaf), "c" (0));
+}
+
 /* Voluntary context switches of a task, from its /proc status file. */
 static int read_nvcsw(const char *status, unsigned long *nvcsw)
 {
@@ -63,6 +72,7 @@ static int read_nvcsw(const char *status, unsigned long *nvcsw)
 struct child_ctx {
 	int result_pipe[2];
 	int stop;
+	int prefixed;
 };
 
 /*
@@ -92,7 +102,10 @@ static int cpuid_child(struct child_ctx *ctx)
 			return 24;
 	}
 
-	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 25;
@@ -326,4 +339,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_file_at(AT_FDCWD, CPUID_OVERRIDE_PATH, self->table), 0);
+
+	pid = spawn_cpuid_child(_metadata, self, &ctx);
+	resume_cpuid_child(_metadata, pid);
+	collect_cpuid_child(_metadata, self, &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 v6 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/20261007-b4-prep-khorenko-vstor-148656-cpuid-v6-0-a8eb906e69b8@virtuozzo.com?part=2


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

Re: [Devel] [PATCH hci-8.1 v6 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)
> 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/20261007-b4-prep-khorenko-vstor-148656-cpuid-v6-0-a8eb906e69b8@virtuozzo.com?part=3


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

Re: [Devel] [PATCH hci-8.1 v6 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/20261007-b4-prep-khorenko-vstor-148656-cpuid-v6-0-a8eb906e69b8@virtuozzo.com?part=4


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

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

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/ve: arch gate runs $(CC) -dumpmachine before lib.mk sets CC
--

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

selftests/ve: check CPUID override emulation for VE tasks

This adds ve_cpuid_override_test, which writes a one-entry
/proc/vz/cpuid_override table and compares what a task in a fresh VE and
the host see for CPUID leaf 1. A second test flushes the table after the
task was armed and expects the hardware value back. The test is added to
the ve selftests Makefile for x86-64 builds only.

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

> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
> index c6efe7c4b4fb..3f494a49270e 100644
> --- a/tools/testing/selftests/ve/Makefile
> +++ b/tools/testing/selftests/ve/Makefile
> @@ -6,4 +6,10 @@ TEST_GEN_PROGS += ve_ns_owner_test
>  TEST_GEN_PROGS += ve_perms_test
>  TEST_GEN_PROGS += ve_mount_accounting_test
>  
> +# CPUID faulting is x86-64 only. ARCH says "x86" for 32-bit too and lib.mk
> +# picks the clang target by its emptiness, so decide by the compiler.
> +ifneq ($(filter x86_64-%,$(shell $(CC) -dumpmachine 2>/dev/null)),)

[Severity: Low]
Is $(CC) the right compiler to query at this point? This ifneq is expanded
while the Makefile is being parsed, before the include of ../lib.mk, and
lib.mk is where the real compiler is assigned:

tools/testing/selftests/lib.mk:
    CC := $(CLANG) $(CLANG_FLAGS) -fintegrated-as    (LLVM builds)
    CC := $(CROSS_COMPILE)gcc                        (otherwise)

Until then $(CC) is make's built-in "cc", unless CC was set on the command
line or in the environment.

For example, with a cross build from an x86_64 host:

    make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-

"cc -dumpmachine" prints x86_64-linux-gnu, so the filter matches and
ve_cpuid_override_test is added to TEST_GEN_PROGS. It is then built with
the aarch64 compiler, where <cpuid.h> and __cpuid_count are not available,
so the whole ve selftest directory fails to build.

The reverse case also looks possible. On an arm64 host cross-building for
x86_64, the test would be skipped.

The comment above the check says it decides "by the compiler", and the
commit message says "The test is built on x86-64 only". Neither seems to
hold for cross builds.

Could the check be done after including lib.mk, or be based on ARCH,
SRCARCH or uname -m instead?

> +TEST_GEN_PROGS += ve_cpuid_override_test
> +endif
> +
>  include ../lib.mk

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

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

Re: [Devel] [PATCH hci-8.1 v6 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/20261007-b4-prep-khorenko-vstor-148656-cpuid-v6-0-a8eb906e69b8@virtuozzo.com?part=5


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

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

[Konstantin Khorenko]

On 10/7/26 14:04, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] selftests/ve: arch gate runs $(CC) -dumpmachine before lib.mk sets CC
> --
> 
> commit de465213924e8ab9123a95613dab45358f011917
> Author: Konstantin Khorenko <khorenko@virtuozzo.com>
> 
> selftests/ve: check CPUID override emulation for VE tasks
> 
> This adds ve_cpuid_override_test, which writes a one-entry
> /proc/vz/cpuid_override table and compares what a task in a fresh VE and
> the host see for CPUID leaf 1. A second test flushes the table after the
> task was armed and expects the hardware value back. The test is added to
> the ve selftests Makefile for x86-64 builds only.
> 
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-148843
> 
>> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
>> index c6efe7c4b4fb..3f494a49270e 100644
>> --- a/tools/testing/selftests/ve/Makefile
>> +++ b/tools/testing/selftests/ve/Makefile
>> @@ -6,4 +6,10 @@ TEST_GEN_PROGS += ve_ns_owner_test
>>  TEST_GEN_PROGS += ve_perms_test
>>  TEST_GEN_PROGS += ve_mount_accounting_test
>>  
>> +# CPUID faulting is x86-64 only. ARCH says "x86" for 32-bit too and lib.mk
>> +# picks the clang target by its emptiness, so decide by the compiler.
>> +ifneq ($(filter x86_64-%,$(shell $(CC) -dumpmachine 2>/dev/null)),)
> 
> [Severity: Low]
> Is $(CC) the right compiler to query at this point? This ifneq is expanded
> while the Makefile is being parsed, before the include of ../lib.mk, and
> lib.mk is where the real compiler is assigned:
> 
> tools/testing/selftests/lib.mk:
>     CC := $(CLANG) $(CLANG_FLAGS) -fintegrated-as    (LLVM builds)
>     CC := $(CROSS_COMPILE)gcc                        (otherwise)
> 
> Until then $(CC) is make's built-in "cc", unless CC was set on the command
> line or in the environment.
> 
> For example, with a cross build from an x86_64 host:
> 
>     make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
> 
> "cc -dumpmachine" prints x86_64-linux-gnu, so the filter matches and
> ve_cpuid_override_test is added to TEST_GEN_PROGS. It is then built with
> the aarch64 compiler, where <cpuid.h> and __cpuid_count are not available,
> so the whole ve selftest directory fails to build.
> 
> The reverse case also looks possible. On an arm64 host cross-building for
> x86_64, the test would be skipped.
> 
> The comment above the check says it decides "by the compiler", and the
> commit message says "The test is built on x86-64 only". Neither seems to
> hold for cross builds.
> 
> Could the check be done after including lib.mk, or be based on ARCH,
> SRCARCH or uname -m instead?

  The observation is right: at that point $(CC) is still make's built-in
  "cc", lib.mk assigns the real compiler only later. The check cannot move
  below the include, though. lib.mk derives its rules from TEST_GEN_PROGS
  while it is being parsed ("all: $(TEST_GEN_PROGS) ..." and the per-program
  rules), so anything added to TEST_GEN_PROGS after the include is simply
  not built.

  The alternatives do not work either:

  - ARCH cannot tell 32-bit from 64-bit x86: the top-level selftests
    Makefile maps both i386 and x86_64 to "x86". Assigning ARCH ourselves
    is out as well, lib.mk uses an empty ARCH to mean "the host target" for
    LLVM=1 builds, and a value it has no CLANG_TARGET_FLAGS_ entry for
    (e.g. "aarch64" from uname -m) makes it error out for the whole ve
    directory. That was the v2 version of this gate, and the review rightly
    rejected it.
  - SRCARCH has the same granularity as ARCH.
  - uname -m knows nothing about cross builds at all, so it is strictly
    worse than asking a compiler.

  What remains is a gap for cross builds only: the gate asks the host
  compiler, so cross-building these selftests from an x86-64 host for
  another architecture would add the test and fail on <cpuid.h>, and the
  reverse would skip it. We do not cross build the vz selftests, the
  feature under test exists on x86-64 only, and I would rather keep a
  three-line gate than grow it to cover a configuration nobody runs. If a
  cross build of tools/testing/selftests/ve ever becomes a thing, the gate
  can be revisited then.

> 
>> +TEST_GEN_PROGS += ve_cpuid_override_test
>> +endif
>> +
>>  include ../lib.mk
> 

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

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

[Vasileios Almpanis]

On Wed, 07 Oct 2026 13:57:01 +0200, Konstantin Khorenko <khorenko@virtuozzo.com> wrote:
> x86/ve: CPUID override follow-ups: arm at once, prefixes, selftests
> 
> 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.
> 
> [...]

Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

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

[Vasileios Almpanis]

> 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)
NIT: Fixes tag? Should be the same as patch 4 no?

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

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

[Konstantin Khorenko]

On 10/8/26 12:18, Vasileios Almpanis wrote:
>> 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)
> NIT: Fixes tag? Should be the same as patch 4 no?

Not exactly, it's for another commit, so i will add during the commit:
Fixes: 50d388697a2c ("x86: make ARCH_[SET|GET]_CPUID friends with /proc/vz/cpuid_override")


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

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

[Konstantin Khorenko]

On Wed, 07 Oct 2026 13:57:01 +0200, Konstantin Khorenko wrote:
> x86/ve: CPUID override follow-ups: arm at once, prefixes, selftests
> 
> 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.
> 
> [...]

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.9.vz10, thanks!

[1/5] selftests/ve: check CPUID override emulation for VE tasks
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/9d42a52a993c
[2/5] x86/ve: enable CPUID faulting at once when the current task enters a VE
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/f2176fcad7e5
[3/5] selftests/ve: check CPUID override right after entering a VE
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/f5881322c62d
[4/5] x86/traps: decode the faulting CPUID with the instruction decoder
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/83f8ff040d0e
[5/5] selftests/ve: check CPUID override emulation for a prefixed CPUID
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/c22db725e448

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>

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