linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress
@ 2024-10-09 15:49 Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header Sean Christopherson
                   ` (14 more replies)
  0 siblings, 15 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

The main purpose of this series is to convert the max_guest_memory_test
into a more generic mmu_stress_test.  The basic gist of the "conversion"
is to have the test do mprotect() on guest memory while vCPUs are
accessing said memory, e.g. to verify KVM and mmu_notifiers are working
as intended.

Patches 1-4 are a somewhat unexpected side quest.  The original plan was
that patch 3 would be a single patch, but things snowballed.

Patch 3 reworks vcpu_get_reg() to return a value instead of using an
out-param.  This is the entire motivation for including these patches;
having to define a variable just to bump the program counter on arm64
annoyed me.

Patch 4 adds hardening to vcpu_{g,s}et_reg() to detect potential
truncation, as KVM's uAPI allows for registers greater than the 64 bits
that are supported in the "outer" selftests APIs ((vcpu_set_reg() takes a
u64, vcpu_get_reg() now returns a u64).

Patch 1 is a change to KVM's uAPI headers to move the KVM_REG_SIZE
definition to common code so that the selftests side of things doesn't
need #ifdefs to implement the hardening in patch 4.

Patch 2 is the truly unexpected part.  With the vcpu_get_reg() rework,
arm64's vpmu_counter_test fails when compiled with gcc-13, and on gcc-11
with an added "noinline".  Long story short, selftests are being compiled
with strict aliasing enabled, which allows the compiler to optimize away
"u64 *" => "uint64_t *" casts as u64 (unsigned long long) and uint64_t
(unsigned long) are technically not aliases of each other.  *sigh*

v3:
 - Rebased onto v6.12-rc2.
 - Disable strict aliasing to fix the PMCR snafu.
 - Collect reviews. [Drew]
 - Minor changelog fixes. [Drew]
 - Include ucall_common.h to prep for RISC-V. [Drew]

v2:
 - Rebase onto kvm/next.
 - Add the aforementioned vcpu_get_reg() changes/disaster.
 - Actually add arm64 support for the fancy mprotect() testcase (I did this
   before v1, but managed to forget to include the changes when posting).
 - Emit "mov %rax, (%rax)" on x86. [James]
 - Add a comment to explain the fancy mprotect() vs. vCPUs logic.
 - Drop the KVM x86 patches (applied and/or will be handled separately).

v1: https://lore.kernel.org/all/20240809194335.1726916-1-seanjc@google.com

Sean Christopherson (14):
  KVM: Move KVM_REG_SIZE() definition to common uAPI header
  KVM: selftests: Disable strict aliasing
  KVM: selftests: Return a value from vcpu_get_reg() instead of using an
    out-param
  KVM: selftests: Assert that vcpu_{g,s}et_reg() won't truncate
  KVM: selftests: Check for a potential unhandled exception iff KVM_RUN
    succeeded
  KVM: selftests: Rename max_guest_memory_test to mmu_stress_test
  KVM: selftests: Only muck with SREGS on x86 in mmu_stress_test
  KVM: selftests: Compute number of extra pages needed in
    mmu_stress_test
  KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c
  KVM: selftests: Enable mmu_stress_test on arm64
  KVM: selftests: Use vcpu_arch_put_guest() in mmu_stress_test
  KVM: selftests: Precisely limit the number of guest loops in
    mmu_stress_test
  KVM: selftests: Add a read-only mprotect() phase to mmu_stress_test
  KVM: selftests: Verify KVM correctly handles mprotect(PROT_READ)

 arch/arm64/include/uapi/asm/kvm.h             |   3 -
 arch/riscv/include/uapi/asm/kvm.h             |   3 -
 include/uapi/linux/kvm.h                      |   4 +
 tools/testing/selftests/kvm/Makefile          |  11 +-
 .../selftests/kvm/aarch64/aarch32_id_regs.c   |  10 +-
 .../selftests/kvm/aarch64/debug-exceptions.c  |   4 +-
 .../selftests/kvm/aarch64/hypercalls.c        |   6 +-
 .../selftests/kvm/aarch64/no-vgic-v3.c        |   2 +-
 .../testing/selftests/kvm/aarch64/psci_test.c |   6 +-
 .../selftests/kvm/aarch64/set_id_regs.c       |  18 +-
 .../kvm/aarch64/vpmu_counter_access.c         |  19 +-
 .../testing/selftests/kvm/include/kvm_util.h  |  10 +-
 .../selftests/kvm/lib/aarch64/processor.c     |   8 +-
 tools/testing/selftests/kvm/lib/kvm_util.c    |   3 +-
 .../selftests/kvm/lib/riscv/processor.c       |  66 +++----
 ..._guest_memory_test.c => mmu_stress_test.c} | 162 ++++++++++++++++--
 .../testing/selftests/kvm/riscv/arch_timer.c  |   2 +-
 .../testing/selftests/kvm/riscv/ebreak_test.c |   2 +-
 .../selftests/kvm/riscv/sbi_pmu_test.c        |   2 +-
 tools/testing/selftests/kvm/s390x/resets.c    |   2 +-
 tools/testing/selftests/kvm/steal_time.c      |   3 +-
 21 files changed, 241 insertions(+), 105 deletions(-)
 rename tools/testing/selftests/kvm/{max_guest_memory_test.c => mmu_stress_test.c} (60%)


base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-10 11:41   ` Anup Patel
  2024-10-09 15:49 ` [PATCH v3 02/14] KVM: selftests: Disable strict aliasing Sean Christopherson
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Define KVM_REG_SIZE() in the common kvm.h header, and delete the arm64 and
RISC-V versions.  As evidenced by the surrounding definitions, all aspects
of the register size encoding are generic, i.e. RISC-V should have moved
arm64's definition to common code instead of copy+pasting.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/arm64/include/uapi/asm/kvm.h | 3 ---
 arch/riscv/include/uapi/asm/kvm.h | 3 ---
 include/uapi/linux/kvm.h          | 4 ++++
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 964df31da975..80b26134e59e 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -43,9 +43,6 @@
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
 #define KVM_DIRTY_LOG_PAGE_OFFSET 64
 
-#define KVM_REG_SIZE(id)						\
-	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
-
 struct kvm_regs {
 	struct user_pt_regs regs;	/* sp = sp_el0 */
 
diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index e97db3296456..4f8d0c04a47b 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -207,9 +207,6 @@ struct kvm_riscv_sbi_sta {
 #define KVM_RISCV_TIMER_STATE_OFF	0
 #define KVM_RISCV_TIMER_STATE_ON	1
 
-#define KVM_REG_SIZE(id)		\
-	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
-
 /* If you need to interpret the index values, here is the key: */
 #define KVM_REG_RISCV_TYPE_MASK		0x00000000FF000000
 #define KVM_REG_RISCV_TYPE_SHIFT	24
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 637efc055145..9deeb13e3e01 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1070,6 +1070,10 @@ struct kvm_dirty_tlb {
 
 #define KVM_REG_SIZE_SHIFT	52
 #define KVM_REG_SIZE_MASK	0x00f0000000000000ULL
+
+#define KVM_REG_SIZE(id)		\
+	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
+
 #define KVM_REG_SIZE_U8		0x0000000000000000ULL
 #define KVM_REG_SIZE_U16	0x0010000000000000ULL
 #define KVM_REG_SIZE_U32	0x0020000000000000ULL
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 02/14] KVM: selftests: Disable strict aliasing
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param Sean Christopherson
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Disable strict aliasing, as has been done in the kernel proper for decades
(literally since before git history) to fix issues where gcc will optimize
away loads in code that looks 100% correct, but is _technically_ undefined
behavior, and thus can be thrown away by the compiler.

E.g. arm64's vPMU counter access test casts a uint64_t (unsigned long)
pointer to a u64 (unsigned long long) pointer when setting PMCR.N via
u64p_replace_bits(), which gcc-13 detects and optimizes away, i.e. ignores
the result and uses the original PMCR.

The issue is most easily observed by making set_pmcr_n() noinline and
wrapping the call with printf(), e.g. sans comments, for this code:

  printf("orig = %lx, next = %lx, want = %lu\n", pmcr_orig, pmcr, pmcr_n);
  set_pmcr_n(&pmcr, pmcr_n);
  printf("orig = %lx, next = %lx, want = %lu\n", pmcr_orig, pmcr, pmcr_n);

gcc-13 generates:

 0000000000401c90 <set_pmcr_n>:
  401c90:       f9400002        ldr     x2, [x0]
  401c94:       b3751022        bfi     x2, x1, #11, #5
  401c98:       f9000002        str     x2, [x0]
  401c9c:       d65f03c0        ret

 0000000000402660 <test_create_vpmu_vm_with_pmcr_n>:
  402724:       aa1403e3        mov     x3, x20
  402728:       aa1503e2        mov     x2, x21
  40272c:       aa1603e0        mov     x0, x22
  402730:       aa1503e1        mov     x1, x21
  402734:       940060ff        bl      41ab30 <_IO_printf>
  402738:       aa1403e1        mov     x1, x20
  40273c:       910183e0        add     x0, sp, #0x60
  402740:       97fffd54        bl      401c90 <set_pmcr_n>
  402744:       aa1403e3        mov     x3, x20
  402748:       aa1503e2        mov     x2, x21
  40274c:       aa1503e1        mov     x1, x21
  402750:       aa1603e0        mov     x0, x22
  402754:       940060f7        bl      41ab30 <_IO_printf>

with the value stored in [sp + 0x60] ignored by both printf() above and
in the test proper, resulting in a false failure due to vcpu_set_reg()
simply storing the original value, not the intended value.

  $ ./vpmu_counter_access
  Random seed: 0x6b8b4567
  orig = 3040, next = 3040, want = 0
  orig = 3040, next = 3040, want = 0
  ==== Test Assertion Failure ====
    aarch64/vpmu_counter_access.c:505: pmcr_n == get_pmcr_n(pmcr)
    pid=71578 tid=71578 errno=9 - Bad file descriptor
       1        0x400673: run_access_test at vpmu_counter_access.c:522
       2         (inlined by) main at vpmu_counter_access.c:643
       3        0x4132d7: __libc_start_call_main at libc-start.o:0
       4        0x413653: __libc_start_main at ??:0
       5        0x40106f: _start at ??:0
    Failed to update PMCR.N to 0 (received: 6)

Somewhat bizarrely, gcc-11 also exhibits the same behavior, but only if
set_pmcr_n() is marked noinline, whereas gcc-13 fails even if set_pmcr_n()
is inlined in its sole caller.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116912
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 960cf6a77198..6246d69d82d7 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -241,10 +241,10 @@ CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
 	-Wno-gnu-variable-sized-type-not-at-end -MD -MP -DCONFIG_64BIT \
 	-fno-builtin-memcmp -fno-builtin-memcpy \
 	-fno-builtin-memset -fno-builtin-strnlen \
-	-fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \
-	-I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \
-	-I$(<D) -Iinclude/$(ARCH_DIR) -I ../rseq -I.. $(EXTRA_CFLAGS) \
-	$(KHDR_INCLUDES)
+	-fno-stack-protector -fno-PIE -fno-strict-aliasing \
+	-I$(LINUX_TOOL_INCLUDE) -I$(LINUX_TOOL_ARCH_INCLUDE) \
+	-I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(ARCH_DIR) \
+	-I ../rseq -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
 ifeq ($(ARCH),s390)
 	CFLAGS += -march=z10
 endif
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 02/14] KVM: selftests: Disable strict aliasing Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-11-01 13:07   ` Mark Brown
  2024-10-09 15:49 ` [PATCH v3 04/14] KVM: selftests: Assert that vcpu_{g,s}et_reg() won't truncate Sean Christopherson
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Return a uint64_t from vcpu_get_reg() instead of having the caller provide
a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests
accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a
64-bit value.  If a use case comes along that needs to get a register that
is larger than 64 bits, then a utility can be added to assert success and
take a void pointer, but until then, forcing an out param yields ugly code
and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg().

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../selftests/kvm/aarch64/aarch32_id_regs.c   | 10 +--
 .../selftests/kvm/aarch64/debug-exceptions.c  |  4 +-
 .../selftests/kvm/aarch64/hypercalls.c        |  6 +-
 .../selftests/kvm/aarch64/no-vgic-v3.c        |  2 +-
 .../testing/selftests/kvm/aarch64/psci_test.c |  6 +-
 .../selftests/kvm/aarch64/set_id_regs.c       | 18 ++---
 .../kvm/aarch64/vpmu_counter_access.c         | 19 +++---
 .../testing/selftests/kvm/include/kvm_util.h  |  6 +-
 .../selftests/kvm/lib/aarch64/processor.c     |  8 +--
 .../selftests/kvm/lib/riscv/processor.c       | 66 +++++++++----------
 .../testing/selftests/kvm/riscv/arch_timer.c  |  2 +-
 .../testing/selftests/kvm/riscv/ebreak_test.c |  2 +-
 .../selftests/kvm/riscv/sbi_pmu_test.c        |  2 +-
 tools/testing/selftests/kvm/s390x/resets.c    |  2 +-
 tools/testing/selftests/kvm/steal_time.c      |  3 +-
 15 files changed, 78 insertions(+), 78 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c b/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c
index 8e5bd07a3727..447d61cae4db 100644
--- a/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c
+++ b/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c
@@ -97,7 +97,7 @@ static void test_user_raz_wi(struct kvm_vcpu *vcpu)
 		uint64_t reg_id = raz_wi_reg_ids[i];
 		uint64_t val;
 
-		vcpu_get_reg(vcpu, reg_id, &val);
+		val = vcpu_get_reg(vcpu, reg_id);
 		TEST_ASSERT_EQ(val, 0);
 
 		/*
@@ -106,7 +106,7 @@ static void test_user_raz_wi(struct kvm_vcpu *vcpu)
 		 */
 		vcpu_set_reg(vcpu, reg_id, BAD_ID_REG_VAL);
 
-		vcpu_get_reg(vcpu, reg_id, &val);
+		val = vcpu_get_reg(vcpu, reg_id);
 		TEST_ASSERT_EQ(val, 0);
 	}
 }
@@ -126,14 +126,14 @@ static void test_user_raz_invariant(struct kvm_vcpu *vcpu)
 		uint64_t reg_id = raz_invariant_reg_ids[i];
 		uint64_t val;
 
-		vcpu_get_reg(vcpu, reg_id, &val);
+		val = vcpu_get_reg(vcpu, reg_id);
 		TEST_ASSERT_EQ(val, 0);
 
 		r = __vcpu_set_reg(vcpu, reg_id, BAD_ID_REG_VAL);
 		TEST_ASSERT(r < 0 && errno == EINVAL,
 			    "unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno);
 
-		vcpu_get_reg(vcpu, reg_id, &val);
+		val = vcpu_get_reg(vcpu, reg_id);
 		TEST_ASSERT_EQ(val, 0);
 	}
 }
@@ -144,7 +144,7 @@ static bool vcpu_aarch64_only(struct kvm_vcpu *vcpu)
 {
 	uint64_t val, el0;
 
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), &val);
+	val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1));
 
 	el0 = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), val);
 	return el0 == ID_AA64PFR0_EL1_ELx_64BIT_ONLY;
diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index 2582c49e525a..b3f3025d2f02 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -501,7 +501,7 @@ void test_single_step_from_userspace(int test_cnt)
 		TEST_ASSERT(ss_enable, "Unexpected KVM_EXIT_DEBUG");
 
 		/* Check if the current pc is expected. */
-		vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc), &pc);
+		pc = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc));
 		TEST_ASSERT(!test_pc || pc == test_pc,
 			    "Unexpected pc 0x%lx (expected 0x%lx)",
 			    pc, test_pc);
@@ -583,7 +583,7 @@ int main(int argc, char *argv[])
 	uint64_t aa64dfr0;
 
 	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &aa64dfr0);
+	aa64dfr0 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1));
 	__TEST_REQUIRE(debug_version(aa64dfr0) >= 6,
 		       "Armv8 debug architecture not supported.");
 	kvm_vm_free(vm);
diff --git a/tools/testing/selftests/kvm/aarch64/hypercalls.c b/tools/testing/selftests/kvm/aarch64/hypercalls.c
index 9d192ce0078d..ec54ec7726e9 100644
--- a/tools/testing/selftests/kvm/aarch64/hypercalls.c
+++ b/tools/testing/selftests/kvm/aarch64/hypercalls.c
@@ -173,7 +173,7 @@ static void test_fw_regs_before_vm_start(struct kvm_vcpu *vcpu)
 		const struct kvm_fw_reg_info *reg_info = &fw_reg_info[i];
 
 		/* First 'read' should be an upper limit of the features supported */
-		vcpu_get_reg(vcpu, reg_info->reg, &val);
+		val = vcpu_get_reg(vcpu, reg_info->reg);
 		TEST_ASSERT(val == FW_REG_ULIMIT_VAL(reg_info->max_feat_bit),
 			"Expected all the features to be set for reg: 0x%lx; expected: 0x%lx; read: 0x%lx",
 			reg_info->reg, FW_REG_ULIMIT_VAL(reg_info->max_feat_bit), val);
@@ -184,7 +184,7 @@ static void test_fw_regs_before_vm_start(struct kvm_vcpu *vcpu)
 			"Failed to clear all the features of reg: 0x%lx; ret: %d",
 			reg_info->reg, errno);
 
-		vcpu_get_reg(vcpu, reg_info->reg, &val);
+		val = vcpu_get_reg(vcpu, reg_info->reg);
 		TEST_ASSERT(val == 0,
 			"Expected all the features to be cleared for reg: 0x%lx", reg_info->reg);
 
@@ -214,7 +214,7 @@ static void test_fw_regs_after_vm_start(struct kvm_vcpu *vcpu)
 		 * Before starting the VM, the test clears all the bits.
 		 * Check if that's still the case.
 		 */
-		vcpu_get_reg(vcpu, reg_info->reg, &val);
+		val = vcpu_get_reg(vcpu, reg_info->reg);
 		TEST_ASSERT(val == 0,
 			"Expected all the features to be cleared for reg: 0x%lx",
 			reg_info->reg);
diff --git a/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c b/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c
index 943d65fc6b0b..f80a519f73bb 100644
--- a/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c
+++ b/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c
@@ -164,7 +164,7 @@ int main(int argc, char *argv[])
 	uint64_t pfr0;
 
 	vm = vm_create_with_one_vcpu(&vcpu, NULL);
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), &pfr0);
+	pfr0 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1));
 	__TEST_REQUIRE(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC), pfr0),
 		       "GICv3 not supported.");
 	kvm_vm_free(vm);
diff --git a/tools/testing/selftests/kvm/aarch64/psci_test.c b/tools/testing/selftests/kvm/aarch64/psci_test.c
index 61731a950def..544ebd2b121b 100644
--- a/tools/testing/selftests/kvm/aarch64/psci_test.c
+++ b/tools/testing/selftests/kvm/aarch64/psci_test.c
@@ -102,8 +102,8 @@ static void assert_vcpu_reset(struct kvm_vcpu *vcpu)
 {
 	uint64_t obs_pc, obs_x0;
 
-	vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc), &obs_pc);
-	vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.regs[0]), &obs_x0);
+	obs_pc = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc));
+	obs_x0 = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.regs[0]));
 
 	TEST_ASSERT(obs_pc == CPU_ON_ENTRY_ADDR,
 		    "unexpected target cpu pc: %lx (expected: %lx)",
@@ -143,7 +143,7 @@ static void host_test_cpu_on(void)
 	 */
 	vcpu_power_off(target);
 
-	vcpu_get_reg(target, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), &target_mpidr);
+	target_mpidr = vcpu_get_reg(target, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1));
 	vcpu_args_set(source, 1, target_mpidr & MPIDR_HWID_BITMASK);
 	enter_guest(source);
 
diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
index 2a3fe7914b72..8a1d92048aef 100644
--- a/tools/testing/selftests/kvm/aarch64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
@@ -336,7 +336,7 @@ static uint64_t test_reg_set_success(struct kvm_vcpu *vcpu, uint64_t reg,
 	uint64_t mask = ftr_bits->mask;
 	uint64_t val, new_val, ftr;
 
-	vcpu_get_reg(vcpu, reg, &val);
+	val = vcpu_get_reg(vcpu, reg);
 	ftr = (val & mask) >> shift;
 
 	ftr = get_safe_value(ftr_bits, ftr);
@@ -346,7 +346,7 @@ static uint64_t test_reg_set_success(struct kvm_vcpu *vcpu, uint64_t reg,
 	val |= ftr;
 
 	vcpu_set_reg(vcpu, reg, val);
-	vcpu_get_reg(vcpu, reg, &new_val);
+	new_val = vcpu_get_reg(vcpu, reg);
 	TEST_ASSERT_EQ(new_val, val);
 
 	return new_val;
@@ -360,7 +360,7 @@ static void test_reg_set_fail(struct kvm_vcpu *vcpu, uint64_t reg,
 	uint64_t val, old_val, ftr;
 	int r;
 
-	vcpu_get_reg(vcpu, reg, &val);
+	val = vcpu_get_reg(vcpu, reg);
 	ftr = (val & mask) >> shift;
 
 	ftr = get_invalid_value(ftr_bits, ftr);
@@ -374,7 +374,7 @@ static void test_reg_set_fail(struct kvm_vcpu *vcpu, uint64_t reg,
 	TEST_ASSERT(r < 0 && errno == EINVAL,
 		    "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno);
 
-	vcpu_get_reg(vcpu, reg, &val);
+	val = vcpu_get_reg(vcpu, reg);
 	TEST_ASSERT_EQ(val, old_val);
 }
 
@@ -471,7 +471,7 @@ static void test_clidr(struct kvm_vcpu *vcpu)
 	uint64_t clidr;
 	int level;
 
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CLIDR_EL1), &clidr);
+	clidr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CLIDR_EL1));
 
 	/* find the first empty level in the cache hierarchy */
 	for (level = 1; level < 7; level++) {
@@ -496,7 +496,7 @@ static void test_ctr(struct kvm_vcpu *vcpu)
 {
 	u64 ctr;
 
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0), &ctr);
+	ctr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0));
 	ctr &= ~CTR_EL0_DIC_MASK;
 	if (ctr & CTR_EL0_IminLine_MASK)
 		ctr--;
@@ -512,7 +512,7 @@ static void test_vcpu_ftr_id_regs(struct kvm_vcpu *vcpu)
 	test_clidr(vcpu);
 	test_ctr(vcpu);
 
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), &val);
+	val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1));
 	val++;
 	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), val);
 
@@ -525,7 +525,7 @@ static void test_assert_id_reg_unchanged(struct kvm_vcpu *vcpu, uint32_t encodin
 	size_t idx = encoding_to_range_idx(encoding);
 	uint64_t observed;
 
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(encoding), &observed);
+	observed = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(encoding));
 	TEST_ASSERT_EQ(test_reg_vals[idx], observed);
 }
 
@@ -560,7 +560,7 @@ int main(void)
 	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
 
 	/* Check for AARCH64 only system */
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), &val);
+	val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1));
 	el0 = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), val);
 	aarch64_only = (el0 == ID_AA64PFR0_EL1_ELx_64BIT_ONLY);
 
diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
index d31b9f64ba14..30d9c9e7ae35 100644
--- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
+++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
@@ -440,8 +440,7 @@ static void create_vpmu_vm(void *guest_code)
 		       "Failed to create vgic-v3, skipping");
 
 	/* Make sure that PMUv3 support is indicated in the ID register */
-	vcpu_get_reg(vpmu_vm.vcpu,
-		     KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &dfr0);
+	dfr0 = vcpu_get_reg(vpmu_vm.vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1));
 	pmuver = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), dfr0);
 	TEST_ASSERT(pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF &&
 		    pmuver >= ID_AA64DFR0_EL1_PMUVer_IMP,
@@ -484,7 +483,7 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail)
 	create_vpmu_vm(guest_code);
 	vcpu = vpmu_vm.vcpu;
 
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), &pmcr_orig);
+	pmcr_orig = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0));
 	pmcr = pmcr_orig;
 
 	/*
@@ -493,7 +492,7 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail)
 	 */
 	set_pmcr_n(&pmcr, pmcr_n);
 	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), pmcr);
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), &pmcr);
+	pmcr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0));
 
 	if (expect_fail)
 		TEST_ASSERT(pmcr_orig == pmcr,
@@ -521,7 +520,7 @@ static void run_access_test(uint64_t pmcr_n)
 	vcpu = vpmu_vm.vcpu;
 
 	/* Save the initial sp to restore them later to run the guest again */
-	vcpu_get_reg(vcpu, ARM64_CORE_REG(sp_el1), &sp);
+	sp = vcpu_get_reg(vcpu, ARM64_CORE_REG(sp_el1));
 
 	run_vcpu(vcpu, pmcr_n);
 
@@ -572,12 +571,12 @@ static void run_pmregs_validity_test(uint64_t pmcr_n)
 		 * Test if the 'set' and 'clr' variants of the registers
 		 * are initialized based on the number of valid counters.
 		 */
-		vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id), &reg_val);
+		reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id));
 		TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0,
 			    "Initial read of set_reg: 0x%llx has unimplemented counters enabled: 0x%lx",
 			    KVM_ARM64_SYS_REG(set_reg_id), reg_val);
 
-		vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id), &reg_val);
+		reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id));
 		TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0,
 			    "Initial read of clr_reg: 0x%llx has unimplemented counters enabled: 0x%lx",
 			    KVM_ARM64_SYS_REG(clr_reg_id), reg_val);
@@ -589,12 +588,12 @@ static void run_pmregs_validity_test(uint64_t pmcr_n)
 		 */
 		vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id), max_counters_mask);
 
-		vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id), &reg_val);
+		reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id));
 		TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0,
 			    "Read of set_reg: 0x%llx has unimplemented counters enabled: 0x%lx",
 			    KVM_ARM64_SYS_REG(set_reg_id), reg_val);
 
-		vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id), &reg_val);
+		reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id));
 		TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0,
 			    "Read of clr_reg: 0x%llx has unimplemented counters enabled: 0x%lx",
 			    KVM_ARM64_SYS_REG(clr_reg_id), reg_val);
@@ -625,7 +624,7 @@ static uint64_t get_pmcr_n_limit(void)
 	uint64_t pmcr;
 
 	create_vpmu_vm(guest_code);
-	vcpu_get_reg(vpmu_vm.vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), &pmcr);
+	pmcr = vcpu_get_reg(vpmu_vm.vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0));
 	destroy_vpmu_vm();
 	return get_pmcr_n(pmcr);
 }
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index bc7c242480d6..287a3ec06df4 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -702,11 +702,13 @@ static inline int __vcpu_set_reg(struct kvm_vcpu *vcpu, uint64_t id, uint64_t va
 
 	return __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
 }
-static inline void vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id, void *addr)
+static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id)
 {
-	struct kvm_one_reg reg = { .id = id, .addr = (uint64_t)addr };
+	uint64_t val;
+	struct kvm_one_reg reg = { .id = id, .addr = (uint64_t)&val };
 
 	vcpu_ioctl(vcpu, KVM_GET_ONE_REG, &reg);
+	return val;
 }
 static inline void vcpu_set_reg(struct kvm_vcpu *vcpu, uint64_t id, uint64_t val)
 {
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index fe4dc3693112..4af94272a758 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -281,8 +281,8 @@ void aarch64_vcpu_setup(struct kvm_vcpu *vcpu, struct kvm_vcpu_init *init)
 	 */
 	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CPACR_EL1), 3 << 20);
 
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1), &sctlr_el1);
-	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL1), &tcr_el1);
+	sctlr_el1 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1));
+	tcr_el1 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL1));
 
 	/* Configure base granule size */
 	switch (vm->mode) {
@@ -360,8 +360,8 @@ void vcpu_arch_dump(FILE *stream, struct kvm_vcpu *vcpu, uint8_t indent)
 {
 	uint64_t pstate, pc;
 
-	vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pstate), &pstate);
-	vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc), &pc);
+	pstate = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pstate));
+	pc = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc));
 
 	fprintf(stream, "%*spstate: 0x%.16lx pc: 0x%.16lx\n",
 		indent, "", pstate, pc);
diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c
index 6ae47b3d6b25..dd663bcf0cc0 100644
--- a/tools/testing/selftests/kvm/lib/riscv/processor.c
+++ b/tools/testing/selftests/kvm/lib/riscv/processor.c
@@ -221,39 +221,39 @@ void vcpu_arch_dump(FILE *stream, struct kvm_vcpu *vcpu, uint8_t indent)
 {
 	struct kvm_riscv_core core;
 
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(mode), &core.mode);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc), &core.regs.pc);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.ra), &core.regs.ra);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.sp), &core.regs.sp);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.gp), &core.regs.gp);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.tp), &core.regs.tp);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t0), &core.regs.t0);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t1), &core.regs.t1);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t2), &core.regs.t2);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s0), &core.regs.s0);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s1), &core.regs.s1);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a0), &core.regs.a0);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a1), &core.regs.a1);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a2), &core.regs.a2);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a3), &core.regs.a3);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a4), &core.regs.a4);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a5), &core.regs.a5);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a6), &core.regs.a6);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a7), &core.regs.a7);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s2), &core.regs.s2);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s3), &core.regs.s3);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s4), &core.regs.s4);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s5), &core.regs.s5);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s6), &core.regs.s6);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s7), &core.regs.s7);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s8), &core.regs.s8);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s9), &core.regs.s9);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s10), &core.regs.s10);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s11), &core.regs.s11);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t3), &core.regs.t3);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t4), &core.regs.t4);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t5), &core.regs.t5);
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t6), &core.regs.t6);
+	core.mode = vcpu_get_reg(vcpu, RISCV_CORE_REG(mode));
+	core.regs.pc = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc));
+	core.regs.ra = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.ra));
+	core.regs.sp = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.sp));
+	core.regs.gp = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.gp));
+	core.regs.tp = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.tp));
+	core.regs.t0 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t0));
+	core.regs.t1 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t1));
+	core.regs.t2 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t2));
+	core.regs.s0 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s0));
+	core.regs.s1 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s1));
+	core.regs.a0 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a0));
+	core.regs.a1 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a1));
+	core.regs.a2 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a2));
+	core.regs.a3 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a3));
+	core.regs.a4 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a4));
+	core.regs.a5 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a5));
+	core.regs.a6 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a6));
+	core.regs.a7 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a7));
+	core.regs.s2 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s2));
+	core.regs.s3 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s3));
+	core.regs.s4 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s4));
+	core.regs.s5 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s5));
+	core.regs.s6 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s6));
+	core.regs.s7 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s7));
+	core.regs.s8 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s8));
+	core.regs.s9 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s9));
+	core.regs.s10 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s10));
+	core.regs.s11 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s11));
+	core.regs.t3 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t3));
+	core.regs.t4 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t4));
+	core.regs.t5 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t5));
+	core.regs.t6 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t6));
 
 	fprintf(stream,
 		" MODE:  0x%lx\n", core.mode);
diff --git a/tools/testing/selftests/kvm/riscv/arch_timer.c b/tools/testing/selftests/kvm/riscv/arch_timer.c
index 2c792228ac0b..9e370800a6a2 100644
--- a/tools/testing/selftests/kvm/riscv/arch_timer.c
+++ b/tools/testing/selftests/kvm/riscv/arch_timer.c
@@ -93,7 +93,7 @@ struct kvm_vm *test_vm_create(void)
 		vcpu_init_vector_tables(vcpus[i]);
 
 	/* Initialize guest timer frequency. */
-	vcpu_get_reg(vcpus[0], RISCV_TIMER_REG(frequency), &timer_freq);
+	timer_freq = vcpu_get_reg(vcpus[0], RISCV_TIMER_REG(frequency));
 	sync_global_to_guest(vm, timer_freq);
 	pr_debug("timer_freq: %lu\n", timer_freq);
 
diff --git a/tools/testing/selftests/kvm/riscv/ebreak_test.c b/tools/testing/selftests/kvm/riscv/ebreak_test.c
index 0e0712854953..cfed6c727bfc 100644
--- a/tools/testing/selftests/kvm/riscv/ebreak_test.c
+++ b/tools/testing/selftests/kvm/riscv/ebreak_test.c
@@ -60,7 +60,7 @@ int main(void)
 
 	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_DEBUG);
 
-	vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc), &pc);
+	pc = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc));
 	TEST_ASSERT_EQ(pc, LABEL_ADDRESS(sw_bp_1));
 
 	/* skip sw_bp_1 */
diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
index f299cbfd23ca..f45c0ecc902d 100644
--- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
+++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
@@ -608,7 +608,7 @@ static void test_vm_events_overflow(void *guest_code)
 
 	vcpu_init_vector_tables(vcpu);
 	/* Initialize guest timer frequency. */
-	vcpu_get_reg(vcpu, RISCV_TIMER_REG(frequency), &timer_freq);
+	timer_freq = vcpu_get_reg(vcpu, RISCV_TIMER_REG(frequency));
 	sync_global_to_guest(vm, timer_freq);
 
 	run_vcpu(vcpu);
diff --git a/tools/testing/selftests/kvm/s390x/resets.c b/tools/testing/selftests/kvm/s390x/resets.c
index 357943f2bea8..b58f75b381e5 100644
--- a/tools/testing/selftests/kvm/s390x/resets.c
+++ b/tools/testing/selftests/kvm/s390x/resets.c
@@ -61,7 +61,7 @@ static void test_one_reg(struct kvm_vcpu *vcpu, uint64_t id, uint64_t value)
 {
 	uint64_t eval_reg;
 
-	vcpu_get_reg(vcpu, id, &eval_reg);
+	eval_reg = vcpu_get_reg(vcpu, id);
 	TEST_ASSERT(eval_reg == value, "value == 0x%lx", value);
 }
 
diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index a8d3afa0b86b..cce2520af720 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -269,9 +269,8 @@ static void guest_code(int cpu)
 static bool is_steal_time_supported(struct kvm_vcpu *vcpu)
 {
 	uint64_t id = RISCV_SBI_EXT_REG(KVM_RISCV_SBI_EXT_STA);
-	unsigned long enabled;
+	unsigned long enabled = vcpu_get_reg(vcpu, id);
 
-	vcpu_get_reg(vcpu, id, &enabled);
 	TEST_ASSERT(enabled == 0 || enabled == 1, "Expected boolean result");
 
 	return enabled;
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 04/14] KVM: selftests: Assert that vcpu_{g,s}et_reg() won't truncate
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (2 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 05/14] KVM: selftests: Check for a potential unhandled exception iff KVM_RUN succeeded Sean Christopherson
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Assert that the register being read/written by vcpu_{g,s}et_reg() is no
larger than a uint64_t, i.e. that a selftest isn't unintentionally
truncating the value being read/written.

Ideally, the assert would be done at compile-time, but that would limit
the checks to hardcoded accesses and/or require fancier compile-time
assertion infrastructure to filter out dynamic usage.

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/kvm_util.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 287a3ec06df4..4c4e5a847f67 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -707,6 +707,8 @@ static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id)
 	uint64_t val;
 	struct kvm_one_reg reg = { .id = id, .addr = (uint64_t)&val };
 
+	TEST_ASSERT(KVM_REG_SIZE(id) <= sizeof(val), "Reg %lx too big", id);
+
 	vcpu_ioctl(vcpu, KVM_GET_ONE_REG, &reg);
 	return val;
 }
@@ -714,6 +716,8 @@ static inline void vcpu_set_reg(struct kvm_vcpu *vcpu, uint64_t id, uint64_t val
 {
 	struct kvm_one_reg reg = { .id = id, .addr = (uint64_t)&val };
 
+	TEST_ASSERT(KVM_REG_SIZE(id) <= sizeof(val), "Reg %lx too big", id);
+
 	vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
 }
 
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 05/14] KVM: selftests: Check for a potential unhandled exception iff KVM_RUN succeeded
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (3 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 04/14] KVM: selftests: Assert that vcpu_{g,s}et_reg() won't truncate Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 06/14] KVM: selftests: Rename max_guest_memory_test to mmu_stress_test Sean Christopherson
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Don't check for an unhandled exception if KVM_RUN failed, e.g. if it
returned errno=EFAULT, as reporting unhandled exceptions is done via a
ucall, i.e. requires KVM_RUN to exit cleanly.  Theoretically, checking
for a ucall on a failed KVM_RUN could get a false positive, e.g. if there
were stale data in vcpu->run from a previous exit.

Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index a2b7df5f1d39..6b3161a0990f 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1646,7 +1646,8 @@ int _vcpu_run(struct kvm_vcpu *vcpu)
 		rc = __vcpu_run(vcpu);
 	} while (rc == -1 && errno == EINTR);
 
-	assert_on_unhandled_exception(vcpu);
+	if (!rc)
+		assert_on_unhandled_exception(vcpu);
 
 	return rc;
 }
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 06/14] KVM: selftests: Rename max_guest_memory_test to mmu_stress_test
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (4 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 05/14] KVM: selftests: Check for a potential unhandled exception iff KVM_RUN succeeded Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 07/14] KVM: selftests: Only muck with SREGS on x86 in mmu_stress_test Sean Christopherson
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Rename max_guest_memory_test to mmu_stress_test so that the name isn't
horribly misleading when future changes extend the test to verify things
like mprotect() interactions, and because the test is useful even when its
configured to populate far less than the maximum amount of guest memory.

Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/Makefile                            | 2 +-
 .../kvm/{max_guest_memory_test.c => mmu_stress_test.c}          | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename tools/testing/selftests/kvm/{max_guest_memory_test.c => mmu_stress_test.c} (100%)

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 6246d69d82d7..8c69a14dc93d 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -139,7 +139,7 @@ TEST_GEN_PROGS_x86_64 += guest_print_test
 TEST_GEN_PROGS_x86_64 += hardware_disable_test
 TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
 TEST_GEN_PROGS_x86_64 += kvm_page_table_test
-TEST_GEN_PROGS_x86_64 += max_guest_memory_test
+TEST_GEN_PROGS_x86_64 += mmu_stress_test
 TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
 TEST_GEN_PROGS_x86_64 += memslot_perf_test
 TEST_GEN_PROGS_x86_64 += rseq_test
diff --git a/tools/testing/selftests/kvm/max_guest_memory_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
similarity index 100%
rename from tools/testing/selftests/kvm/max_guest_memory_test.c
rename to tools/testing/selftests/kvm/mmu_stress_test.c
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 07/14] KVM: selftests: Only muck with SREGS on x86 in mmu_stress_test
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (5 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 06/14] KVM: selftests: Rename max_guest_memory_test to mmu_stress_test Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 08/14] KVM: selftests: Compute number of extra pages needed " Sean Christopherson
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Try to get/set SREGS in mmu_stress_test only when running on x86, as the
ioctls are supported only by x86 and PPC, and the latter doesn't yet
support KVM selftests.

Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index 0b9678858b6d..847da23ec1b1 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -59,10 +59,10 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
 
 static void *vcpu_worker(void *data)
 {
+	struct kvm_sregs __maybe_unused sregs;
 	struct vcpu_info *info = data;
 	struct kvm_vcpu *vcpu = info->vcpu;
 	struct kvm_vm *vm = vcpu->vm;
-	struct kvm_sregs sregs;
 
 	vcpu_args_set(vcpu, 3, info->start_gpa, info->end_gpa, vm->page_size);
 
@@ -70,12 +70,12 @@ static void *vcpu_worker(void *data)
 
 	run_vcpu(vcpu);
 	rendezvous_with_boss();
+#ifdef __x86_64__
 	vcpu_sregs_get(vcpu, &sregs);
-#ifdef __x86_64__
 	/* Toggle CR0.WP to trigger a MMU context reset. */
 	sregs.cr0 ^= X86_CR0_WP;
-#endif
 	vcpu_sregs_set(vcpu, &sregs);
+#endif
 	rendezvous_with_boss();
 
 	run_vcpu(vcpu);
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 08/14] KVM: selftests: Compute number of extra pages needed in mmu_stress_test
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (6 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 07/14] KVM: selftests: Only muck with SREGS on x86 in mmu_stress_test Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 09/14] KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c Sean Christopherson
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Create mmu_stress_tests's VM with the correct number of extra pages needed
to map all of memory in the guest.  The bug hasn't been noticed before as
the test currently runs only on x86, which maps guest memory with 1GiB
pages, i.e. doesn't need much memory in the guest for page tables.

Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index 847da23ec1b1..5467b12f5903 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -209,7 +209,13 @@ int main(int argc, char *argv[])
 	vcpus = malloc(nr_vcpus * sizeof(*vcpus));
 	TEST_ASSERT(vcpus, "Failed to allocate vCPU array");
 
-	vm = vm_create_with_vcpus(nr_vcpus, guest_code, vcpus);
+	vm = __vm_create_with_vcpus(VM_SHAPE_DEFAULT, nr_vcpus,
+#ifdef __x86_64__
+				    max_mem / SZ_1G,
+#else
+				    max_mem / vm_guest_mode_params[VM_MODE_DEFAULT].page_size,
+#endif
+				    guest_code, vcpus);
 
 	max_gpa = vm->max_gfn << vm->page_shift;
 	TEST_ASSERT(max_gpa > (4 * slot_size), "MAXPHYADDR <4gb ");
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 09/14] KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (7 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 08/14] KVM: selftests: Compute number of extra pages needed " Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
       [not found]   ` <CADrL8HUEwnP8e700y2XYDgVhhUJuj1UEJmd2NLdtZ1dV845DNw@mail.gmail.com>
  2024-10-09 15:49 ` [PATCH v3 10/14] KVM: selftests: Enable mmu_stress_test on arm64 Sean Christopherson
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Explicitly include ucall_common.h in the MMU stress test, as unlike arm64
and x86-64, RISC-V doesn't include ucall_common.h in its processor.h, i.e.
this will allow enabling the test on RISC-V.

Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index 5467b12f5903..fbb693428a82 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -15,6 +15,7 @@
 #include "test_util.h"
 #include "guest_modes.h"
 #include "processor.h"
+#include "ucall_common.h"
 
 static void guest_code(uint64_t start_gpa, uint64_t end_gpa, uint64_t stride)
 {
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 10/14] KVM: selftests: Enable mmu_stress_test on arm64
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (8 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 09/14] KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 11/14] KVM: selftests: Use vcpu_arch_put_guest() in mmu_stress_test Sean Christopherson
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Enable the mmu_stress_test on arm64.  The intent was to enable the test
across all architectures when it was first added, but a few goofs made it
unrunnable on !x86.  Now that those goofs are fixed, at least for arm64,
enable the test.

Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Marc Zyngier <maz@kernel.org>
Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 8c69a14dc93d..4db74792d689 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -178,6 +178,7 @@ TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
 TEST_GEN_PROGS_aarch64 += kvm_page_table_test
 TEST_GEN_PROGS_aarch64 += memslot_modification_stress_test
 TEST_GEN_PROGS_aarch64 += memslot_perf_test
+TEST_GEN_PROGS_aarch64 += mmu_stress_test
 TEST_GEN_PROGS_aarch64 += rseq_test
 TEST_GEN_PROGS_aarch64 += set_memory_region_test
 TEST_GEN_PROGS_aarch64 += steal_time
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 11/14] KVM: selftests: Use vcpu_arch_put_guest() in mmu_stress_test
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (9 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 10/14] KVM: selftests: Enable mmu_stress_test on arm64 Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 12/14] KVM: selftests: Precisely limit the number of guest loops " Sean Christopherson
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Use vcpu_arch_put_guest() to write memory from the guest in
mmu_stress_test as an easy way to provide a bit of extra coverage.

Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index fbb693428a82..656a837c7f49 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -23,7 +23,7 @@ static void guest_code(uint64_t start_gpa, uint64_t end_gpa, uint64_t stride)
 
 	for (;;) {
 		for (gpa = start_gpa; gpa < end_gpa; gpa += stride)
-			*((volatile uint64_t *)gpa) = gpa;
+			vcpu_arch_put_guest(*((volatile uint64_t *)gpa), gpa);
 		GUEST_SYNC(0);
 	}
 }
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 12/14] KVM: selftests: Precisely limit the number of guest loops in mmu_stress_test
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (10 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 11/14] KVM: selftests: Use vcpu_arch_put_guest() in mmu_stress_test Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 13/14] KVM: selftests: Add a read-only mprotect() phase to mmu_stress_test Sean Christopherson
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Run the exact number of guest loops required in mmu_stress_test instead
of looping indefinitely in anticipation of adding more stages that run
different code (e.g. reads instead of writes).

Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 25 ++++++++++++++-----
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index 656a837c7f49..c6bf18cb7c89 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -20,12 +20,15 @@
 static void guest_code(uint64_t start_gpa, uint64_t end_gpa, uint64_t stride)
 {
 	uint64_t gpa;
+	int i;
 
-	for (;;) {
+	for (i = 0; i < 2; i++) {
 		for (gpa = start_gpa; gpa < end_gpa; gpa += stride)
 			vcpu_arch_put_guest(*((volatile uint64_t *)gpa), gpa);
-		GUEST_SYNC(0);
+		GUEST_SYNC(i);
 	}
+
+	GUEST_ASSERT(0);
 }
 
 struct vcpu_info {
@@ -52,10 +55,18 @@ static void rendezvous_with_boss(void)
 	}
 }
 
-static void run_vcpu(struct kvm_vcpu *vcpu)
+static void assert_sync_stage(struct kvm_vcpu *vcpu, int stage)
+{
+	struct ucall uc;
+
+	TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_SYNC);
+	TEST_ASSERT_EQ(uc.args[1], stage);
+}
+
+static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
 {
 	vcpu_run(vcpu);
-	TEST_ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_SYNC);
+	assert_sync_stage(vcpu, stage);
 }
 
 static void *vcpu_worker(void *data)
@@ -69,7 +80,8 @@ static void *vcpu_worker(void *data)
 
 	rendezvous_with_boss();
 
-	run_vcpu(vcpu);
+	/* Stage 0, write all of guest memory. */
+	run_vcpu(vcpu, 0);
 	rendezvous_with_boss();
 #ifdef __x86_64__
 	vcpu_sregs_get(vcpu, &sregs);
@@ -79,7 +91,8 @@ static void *vcpu_worker(void *data)
 #endif
 	rendezvous_with_boss();
 
-	run_vcpu(vcpu);
+	/* Stage 1, re-write all of guest memory. */
+	run_vcpu(vcpu, 1);
 	rendezvous_with_boss();
 
 	return NULL;
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 13/14] KVM: selftests: Add a read-only mprotect() phase to mmu_stress_test
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (11 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 12/14] KVM: selftests: Precisely limit the number of guest loops " Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-09 15:49 ` [PATCH v3 14/14] KVM: selftests: Verify KVM correctly handles mprotect(PROT_READ) Sean Christopherson
  2024-10-31 19:51 ` [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Add a third phase of mmu_stress_test to verify that mprotect()ing guest
memory to make it read-only doesn't cause explosions, e.g. to verify KVM
correctly handles the resulting mmu_notifier invalidations.

Reviewed-by: James Houghton <jthoughton@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 22 +++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index c6bf18cb7c89..0918fade9267 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -28,6 +28,10 @@ static void guest_code(uint64_t start_gpa, uint64_t end_gpa, uint64_t stride)
 		GUEST_SYNC(i);
 	}
 
+	for (gpa = start_gpa; gpa < end_gpa; gpa += stride)
+		*((volatile uint64_t *)gpa);
+	GUEST_SYNC(2);
+
 	GUEST_ASSERT(0);
 }
 
@@ -95,6 +99,10 @@ static void *vcpu_worker(void *data)
 	run_vcpu(vcpu, 1);
 	rendezvous_with_boss();
 
+	/* Stage 2, read all of guest memory, which is now read-only. */
+	run_vcpu(vcpu, 2);
+	rendezvous_with_boss();
+
 	return NULL;
 }
 
@@ -175,7 +183,7 @@ int main(int argc, char *argv[])
 	const uint64_t start_gpa = SZ_4G;
 	const int first_slot = 1;
 
-	struct timespec time_start, time_run1, time_reset, time_run2;
+	struct timespec time_start, time_run1, time_reset, time_run2, time_ro;
 	uint64_t max_gpa, gpa, slot_size, max_mem, i;
 	int max_slots, slot, opt, fd;
 	bool hugepages = false;
@@ -279,14 +287,20 @@ int main(int argc, char *argv[])
 	rendezvous_with_vcpus(&time_reset, "reset");
 	rendezvous_with_vcpus(&time_run2, "run 2");
 
+	mprotect(mem, slot_size, PROT_READ);
+	rendezvous_with_vcpus(&time_ro, "mprotect RO");
+
+	time_ro    = timespec_sub(time_ro,     time_run2);
 	time_run2  = timespec_sub(time_run2,   time_reset);
-	time_reset = timespec_sub(time_reset, time_run1);
+	time_reset = timespec_sub(time_reset,  time_run1);
 	time_run1  = timespec_sub(time_run1,   time_start);
 
-	pr_info("run1 = %ld.%.9lds, reset = %ld.%.9lds, run2 =  %ld.%.9lds\n",
+	pr_info("run1 = %ld.%.9lds, reset = %ld.%.9lds, run2 = %ld.%.9lds, "
+		"ro = %ld.%.9lds\n",
 		time_run1.tv_sec, time_run1.tv_nsec,
 		time_reset.tv_sec, time_reset.tv_nsec,
-		time_run2.tv_sec, time_run2.tv_nsec);
+		time_run2.tv_sec, time_run2.tv_nsec,
+		time_ro.tv_sec, time_ro.tv_nsec);
 
 	/*
 	 * Delete even numbered slots (arbitrary) and unmap the first half of
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* [PATCH v3 14/14] KVM: selftests: Verify KVM correctly handles mprotect(PROT_READ)
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (12 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 13/14] KVM: selftests: Add a read-only mprotect() phase to mmu_stress_test Sean Christopherson
@ 2024-10-09 15:49 ` Sean Christopherson
  2024-10-31 19:51 ` [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
  14 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-09 15:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Sean Christopherson, Andrew Jones, James Houghton

Add two phases to mmu_stress_test to verify that KVM correctly handles
guest memory that was writable, and then made read-only in the primary MMU,
and then made writable again.

Add bonus coverage for x86 and arm64 to verify that all of guest memory was
marked read-only.  Making forward progress (without making memory writable)
requires arch specific code to skip over the faulting instruction, but the
test can at least verify each vCPU's starting page was made read-only for
other architectures.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 104 +++++++++++++++++-
 1 file changed, 101 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index 0918fade9267..d9c76b4c0d88 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -17,6 +17,8 @@
 #include "processor.h"
 #include "ucall_common.h"
 
+static bool mprotect_ro_done;
+
 static void guest_code(uint64_t start_gpa, uint64_t end_gpa, uint64_t stride)
 {
 	uint64_t gpa;
@@ -32,6 +34,42 @@ static void guest_code(uint64_t start_gpa, uint64_t end_gpa, uint64_t stride)
 		*((volatile uint64_t *)gpa);
 	GUEST_SYNC(2);
 
+	/*
+	 * Write to the region while mprotect(PROT_READ) is underway.  Keep
+	 * looping until the memory is guaranteed to be read-only, otherwise
+	 * vCPUs may complete their writes and advance to the next stage
+	 * prematurely.
+	 *
+	 * For architectures that support skipping the faulting instruction,
+	 * generate the store via inline assembly to ensure the exact length
+	 * of the instruction is known and stable (vcpu_arch_put_guest() on
+	 * fixed-length architectures should work, but the cost of paranoia
+	 * is low in this case).  For x86, hand-code the exact opcode so that
+	 * there is no room for variability in the generated instruction.
+	 */
+	do {
+		for (gpa = start_gpa; gpa < end_gpa; gpa += stride)
+#ifdef __x86_64__
+			asm volatile(".byte 0x48,0x89,0x00" :: "a"(gpa) : "memory"); /* mov %rax, (%rax) */
+#elif defined(__aarch64__)
+			asm volatile("str %0, [%0]" :: "r" (gpa) : "memory");
+#else
+			vcpu_arch_put_guest(*((volatile uint64_t *)gpa), gpa);
+#endif
+	} while (!READ_ONCE(mprotect_ro_done));
+
+	/*
+	 * Only architectures that write the entire range can explicitly sync,
+	 * as other architectures will be stuck on the write fault.
+	 */
+#if defined(__x86_64__) || defined(__aarch64__)
+	GUEST_SYNC(3);
+#endif
+
+	for (gpa = start_gpa; gpa < end_gpa; gpa += stride)
+		vcpu_arch_put_guest(*((volatile uint64_t *)gpa), gpa);
+	GUEST_SYNC(4);
+
 	GUEST_ASSERT(0);
 }
 
@@ -79,6 +117,7 @@ static void *vcpu_worker(void *data)
 	struct vcpu_info *info = data;
 	struct kvm_vcpu *vcpu = info->vcpu;
 	struct kvm_vm *vm = vcpu->vm;
+	int r;
 
 	vcpu_args_set(vcpu, 3, info->start_gpa, info->end_gpa, vm->page_size);
 
@@ -101,6 +140,57 @@ static void *vcpu_worker(void *data)
 
 	/* Stage 2, read all of guest memory, which is now read-only. */
 	run_vcpu(vcpu, 2);
+
+	/*
+	 * Stage 3, write guest memory and verify KVM returns -EFAULT for once
+	 * the mprotect(PROT_READ) lands.  Only architectures that support
+	 * validating *all* of guest memory sync for this stage, as vCPUs will
+	 * be stuck on the faulting instruction for other architectures.  Go to
+	 * stage 3 without a rendezvous
+	 */
+	do {
+		r = _vcpu_run(vcpu);
+	} while (!r);
+	TEST_ASSERT(r == -1 && errno == EFAULT,
+		    "Expected EFAULT on write to RO memory, got r = %d, errno = %d", r, errno);
+
+#if defined(__x86_64__) || defined(__aarch64__)
+	/*
+	 * Verify *all* writes from the guest hit EFAULT due to the VMA now
+	 * being read-only.  x86 and arm64 only at this time as skipping the
+	 * instruction that hits the EFAULT requires advancing the program
+	 * counter, which is arch specific and relies on inline assembly.
+	 */
+#ifdef __x86_64__
+	vcpu->run->kvm_valid_regs = KVM_SYNC_X86_REGS;
+#endif
+	for (;;) {
+		r = _vcpu_run(vcpu);
+		if (!r)
+			break;
+		TEST_ASSERT_EQ(errno, EFAULT);
+#if defined(__x86_64__)
+		WRITE_ONCE(vcpu->run->kvm_dirty_regs, KVM_SYNC_X86_REGS);
+		vcpu->run->s.regs.regs.rip += 3;
+#elif defined(__aarch64__)
+		vcpu_set_reg(vcpu, ARM64_CORE_REG(regs.pc),
+			     vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc)) + 4);
+#endif
+
+	}
+	assert_sync_stage(vcpu, 3);
+#endif /* __x86_64__ || __aarch64__ */
+	rendezvous_with_boss();
+
+	/*
+	 * Stage 4.  Run to completion, waiting for mprotect(PROT_WRITE) to
+	 * make the memory writable again.
+	 */
+	do {
+		r = _vcpu_run(vcpu);
+	} while (r && errno == EFAULT);
+	TEST_ASSERT_EQ(r, 0);
+	assert_sync_stage(vcpu, 4);
 	rendezvous_with_boss();
 
 	return NULL;
@@ -183,7 +273,7 @@ int main(int argc, char *argv[])
 	const uint64_t start_gpa = SZ_4G;
 	const int first_slot = 1;
 
-	struct timespec time_start, time_run1, time_reset, time_run2, time_ro;
+	struct timespec time_start, time_run1, time_reset, time_run2, time_ro, time_rw;
 	uint64_t max_gpa, gpa, slot_size, max_mem, i;
 	int max_slots, slot, opt, fd;
 	bool hugepages = false;
@@ -288,19 +378,27 @@ int main(int argc, char *argv[])
 	rendezvous_with_vcpus(&time_run2, "run 2");
 
 	mprotect(mem, slot_size, PROT_READ);
+	usleep(10);
+	mprotect_ro_done = true;
+	sync_global_to_guest(vm, mprotect_ro_done);
+
 	rendezvous_with_vcpus(&time_ro, "mprotect RO");
+	mprotect(mem, slot_size, PROT_READ | PROT_WRITE);
+	rendezvous_with_vcpus(&time_rw, "mprotect RW");
 
+	time_rw    = timespec_sub(time_rw,     time_ro);
 	time_ro    = timespec_sub(time_ro,     time_run2);
 	time_run2  = timespec_sub(time_run2,   time_reset);
 	time_reset = timespec_sub(time_reset,  time_run1);
 	time_run1  = timespec_sub(time_run1,   time_start);
 
 	pr_info("run1 = %ld.%.9lds, reset = %ld.%.9lds, run2 = %ld.%.9lds, "
-		"ro = %ld.%.9lds\n",
+		"ro = %ld.%.9lds, rw = %ld.%.9lds\n",
 		time_run1.tv_sec, time_run1.tv_nsec,
 		time_reset.tv_sec, time_reset.tv_nsec,
 		time_run2.tv_sec, time_run2.tv_nsec,
-		time_ro.tv_sec, time_ro.tv_nsec);
+		time_ro.tv_sec, time_ro.tv_nsec,
+		time_rw.tv_sec, time_rw.tv_nsec);
 
 	/*
 	 * Delete even numbered slots (arbitrary) and unmap the first half of
-- 
2.47.0.rc0.187.ge670bccf7e-goog



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

* Re: [PATCH v3 01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header
  2024-10-09 15:49 ` [PATCH v3 01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header Sean Christopherson
@ 2024-10-10 11:41   ` Anup Patel
  0 siblings, 0 replies; 28+ messages in thread
From: Anup Patel @ 2024-10-10 11:41 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Marc Zyngier, Oliver Upton, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Paolo Bonzini, Christian Borntraeger, Janosch Frank,
	Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm, kvm-riscv,
	linux-riscv, linux-kernel, Andrew Jones, James Houghton

On Wed, Oct 9, 2024 at 9:19 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Define KVM_REG_SIZE() in the common kvm.h header, and delete the arm64 and
> RISC-V versions.  As evidenced by the surrounding definitions, all aspects
> of the register size encoding are generic, i.e. RISC-V should have moved
> arm64's definition to common code instead of copy+pasting.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

For KVM RISC-V:
Acked-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> ---
>  arch/arm64/include/uapi/asm/kvm.h | 3 ---
>  arch/riscv/include/uapi/asm/kvm.h | 3 ---
>  include/uapi/linux/kvm.h          | 4 ++++
>  3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 964df31da975..80b26134e59e 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -43,9 +43,6 @@
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>  #define KVM_DIRTY_LOG_PAGE_OFFSET 64
>
> -#define KVM_REG_SIZE(id)                                               \
> -       (1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
> -
>  struct kvm_regs {
>         struct user_pt_regs regs;       /* sp = sp_el0 */
>
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index e97db3296456..4f8d0c04a47b 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -207,9 +207,6 @@ struct kvm_riscv_sbi_sta {
>  #define KVM_RISCV_TIMER_STATE_OFF      0
>  #define KVM_RISCV_TIMER_STATE_ON       1
>
> -#define KVM_REG_SIZE(id)               \
> -       (1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
> -
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_RISCV_TYPE_MASK                0x00000000FF000000
>  #define KVM_REG_RISCV_TYPE_SHIFT       24
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 637efc055145..9deeb13e3e01 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1070,6 +1070,10 @@ struct kvm_dirty_tlb {
>
>  #define KVM_REG_SIZE_SHIFT     52
>  #define KVM_REG_SIZE_MASK      0x00f0000000000000ULL
> +
> +#define KVM_REG_SIZE(id)               \
> +       (1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
> +
>  #define KVM_REG_SIZE_U8                0x0000000000000000ULL
>  #define KVM_REG_SIZE_U16       0x0010000000000000ULL
>  #define KVM_REG_SIZE_U32       0x0020000000000000ULL
> --
> 2.47.0.rc0.187.ge670bccf7e-goog
>


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

* Re: [PATCH v3 09/14] KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c
       [not found]   ` <CADrL8HUEwnP8e700y2XYDgVhhUJuj1UEJmd2NLdtZ1dV845DNw@mail.gmail.com>
@ 2024-10-28 20:45     ` Sean Christopherson
  0 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-10-28 20:45 UTC (permalink / raw)
  To: James Houghton
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel

+lists

Unless there is a good reason for off-list communication, please keep the Cc
list intact when responding, even for trivialities.

On Mon, Oct 28, 2024, James Houghton wrote:
> BTW, there is a typo in the shortlog: "sefltests". Guessing you've
> noticed that already.

I had not.  Thanks for the eyeballs!


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

* Re: [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress
  2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
                   ` (13 preceding siblings ...)
  2024-10-09 15:49 ` [PATCH v3 14/14] KVM: selftests: Verify KVM correctly handles mprotect(PROT_READ) Sean Christopherson
@ 2024-10-31 19:51 ` Sean Christopherson
  2024-11-05  5:53   ` Sean Christopherson
  14 siblings, 1 reply; 28+ messages in thread
From: Sean Christopherson @ 2024-10-31 19:51 UTC (permalink / raw)
  To: Sean Christopherson, Marc Zyngier, Oliver Upton, Anup Patel,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Paolo Bonzini,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Andrew Jones, James Houghton

On Wed, 09 Oct 2024 08:49:39 -0700, Sean Christopherson wrote:
> The main purpose of this series is to convert the max_guest_memory_test
> into a more generic mmu_stress_test.  The basic gist of the "conversion"
> is to have the test do mprotect() on guest memory while vCPUs are
> accessing said memory, e.g. to verify KVM and mmu_notifiers are working
> as intended.
> 
> Patches 1-4 are a somewhat unexpected side quest.  The original plan was
> that patch 3 would be a single patch, but things snowballed.
> 
> [...]

Applied to kvm-x86 selftests, with the typo fixup pointed out by James.  Thanks!

[01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header
        https://github.com/kvm-x86/linux/commit/5e07fd0bf516
[02/14] KVM: selftests: Disable strict aliasing
        https://github.com/kvm-x86/linux/commit/d1ce2bcd8d2e
[03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
        https://github.com/kvm-x86/linux/commit/5c6c7b71a45c
[04/14] KVM: selftests: Assert that vcpu_{g,s}et_reg() won't truncate
        https://github.com/kvm-x86/linux/commit/6aa2df3eb90b
[05/14] KVM: selftests: Check for a potential unhandled exception iff KVM_RUN succeeded
        https://github.com/kvm-x86/linux/commit/be9f2746d20b
[06/14] KVM: selftests: Rename max_guest_memory_test to mmu_stress_test
        https://github.com/kvm-x86/linux/commit/06694f27cfcc
[07/14] KVM: selftests: Only muck with SREGS on x86 in mmu_stress_test
        https://github.com/kvm-x86/linux/commit/8556ce365a07
[08/14] KVM: selftests: Compute number of extra pages needed in mmu_stress_test
        https://github.com/kvm-x86/linux/commit/c7b7876ac5d4
[09/14] KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c
        https://github.com/kvm-x86/linux/commit/a657856469e1
[10/14] KVM: selftests: Enable mmu_stress_test on arm64
        https://github.com/kvm-x86/linux/commit/1e53cde06102
[11/14] KVM: selftests: Use vcpu_arch_put_guest() in mmu_stress_test
        https://github.com/kvm-x86/linux/commit/8630563012b9
[12/14] KVM: selftests: Precisely limit the number of guest loops in mmu_stress_test
        https://github.com/kvm-x86/linux/commit/3d4585c220dc
[13/14] KVM: selftests: Add a read-only mprotect() phase to mmu_stress_test
        https://github.com/kvm-x86/linux/commit/eaafeebca75a
[14/14] KVM: selftests: Verify KVM correctly handles mprotect(PROT_READ)
        https://github.com/kvm-x86/linux/commit/a3cd5c187742

--
https://github.com/kvm-x86/linux/tree/next


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-10-09 15:49 ` [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param Sean Christopherson
@ 2024-11-01 13:07   ` Mark Brown
  2024-11-01 14:48     ` Sean Christopherson
  0 siblings, 1 reply; 28+ messages in thread
From: Mark Brown @ 2024-11-01 13:07 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

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

On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote:
> Return a uint64_t from vcpu_get_reg() instead of having the caller provide
> a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests
> accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a
> 64-bit value.  If a use case comes along that needs to get a register that
> is larger than 64 bits, then a utility can be added to assert success and
> take a void pointer, but until then, forcing an out param yields ugly code
> and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg().

This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the
build on arm64:

aarch64/psci_test.c: In function ‘host_test_system_off2’:
aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’
  247 |         vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version);
      |         ^~~~~~~~~~~~
In file included from aarch64/psci_test.c:18:
include/kvm_util.h:705:24: note: declared here
  705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id)
      |                        ^~~~~~~~~~~~
At top level:
cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at
-end’ may have been intended to silence earlier diagnostics

since the updates done to that file did not take account of 72be5aa6be4
("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged
in the kvm-arm64 tree.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 13:07   ` Mark Brown
@ 2024-11-01 14:48     ` Sean Christopherson
  2024-11-01 15:38       ` Oliver Upton
  0 siblings, 1 reply; 28+ messages in thread
From: Sean Christopherson @ 2024-11-01 14:48 UTC (permalink / raw)
  To: Mark Brown
  Cc: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

On Fri, Nov 01, 2024, Mark Brown wrote:
> On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote:
> > Return a uint64_t from vcpu_get_reg() instead of having the caller provide
> > a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests
> > accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a
> > 64-bit value.  If a use case comes along that needs to get a register that
> > is larger than 64 bits, then a utility can be added to assert success and
> > take a void pointer, but until then, forcing an out param yields ugly code
> > and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg().
> 
> This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the
> build on arm64:
> 
> aarch64/psci_test.c: In function ‘host_test_system_off2’:
> aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’
>   247 |         vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version);
>       |         ^~~~~~~~~~~~
> In file included from aarch64/psci_test.c:18:
> include/kvm_util.h:705:24: note: declared here
>   705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id)
>       |                        ^~~~~~~~~~~~
> At top level:
> cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at
> -end’ may have been intended to silence earlier diagnostics
> 
> since the updates done to that file did not take account of 72be5aa6be4
> ("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged
> in the kvm-arm64 tree.

Bugger.  In hindsight, it's obvious that of course arch selftests would add usage
of vcpu_get_reg().

Unless someone has a better idea, I'll drop the series from kvm-x86, post a new
version that applies on linux-next, and then re-apply the series just before the
v6.13 merge window (rinse and repeat as needed if more vcpu_get_reg() users come
along).

That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g.
have a "selftests_late" or whatever topic branch.

Sorry for the pain Mark, you've been playing janitor for us too much lately.

[*] https://lore.kernel.org/all/20240826190116.145945-1-seanjc@google.com


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 14:48     ` Sean Christopherson
@ 2024-11-01 15:38       ` Oliver Upton
  2024-11-01 15:59         ` Sean Christopherson
  0 siblings, 1 reply; 28+ messages in thread
From: Oliver Upton @ 2024-11-01 15:38 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Mark Brown, Marc Zyngier, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

Hey,

On Fri, Nov 01, 2024 at 07:48:00AM -0700, Sean Christopherson wrote:
> On Fri, Nov 01, 2024, Mark Brown wrote:
> > On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote:
> > > Return a uint64_t from vcpu_get_reg() instead of having the caller provide
> > > a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests
> > > accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a
> > > 64-bit value.  If a use case comes along that needs to get a register that
> > > is larger than 64 bits, then a utility can be added to assert success and
> > > take a void pointer, but until then, forcing an out param yields ugly code
> > > and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg().
> > 
> > This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the
> > build on arm64:
> > 
> > aarch64/psci_test.c: In function ‘host_test_system_off2’:
> > aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’
> >   247 |         vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version);
> >       |         ^~~~~~~~~~~~
> > In file included from aarch64/psci_test.c:18:
> > include/kvm_util.h:705:24: note: declared here
> >   705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id)
> >       |                        ^~~~~~~~~~~~
> > At top level:
> > cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at
> > -end’ may have been intended to silence earlier diagnostics
> > 
> > since the updates done to that file did not take account of 72be5aa6be4
> > ("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged
> > in the kvm-arm64 tree.
> 
> Bugger.  In hindsight, it's obvious that of course arch selftests would add usage
> of vcpu_get_reg().
> 
> Unless someone has a better idea, I'll drop the series from kvm-x86, post a new
> version that applies on linux-next, and then re-apply the series just before the
> v6.13 merge window (rinse and repeat as needed if more vcpu_get_reg() users come
> along).

Can you instead just push out a topic branch and let the affected
maintainers deal with it? This is the usual way we handle conflicts
between trees...

> That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g.
> have a "selftests_late" or whatever topic branch.

The right time to do KVM-wide changes (even selftests) is *early* in the
development cycle, not last minute. It gives us plenty of time to iron out
the wrinkles.

> Sorry for the pain Mark, you've been playing janitor for us too much lately.

+1, appreciate your help on this.

-- 
Thanks,
Oliver


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 15:38       ` Oliver Upton
@ 2024-11-01 15:59         ` Sean Christopherson
  2024-11-01 16:11           ` Oliver Upton
  0 siblings, 1 reply; 28+ messages in thread
From: Sean Christopherson @ 2024-11-01 15:59 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Mark Brown, Marc Zyngier, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

On Fri, Nov 01, 2024, Oliver Upton wrote:
> Hey,
> 
> On Fri, Nov 01, 2024 at 07:48:00AM -0700, Sean Christopherson wrote:
> > On Fri, Nov 01, 2024, Mark Brown wrote:
> > > On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote:
> > > > Return a uint64_t from vcpu_get_reg() instead of having the caller provide
> > > > a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests
> > > > accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a
> > > > 64-bit value.  If a use case comes along that needs to get a register that
> > > > is larger than 64 bits, then a utility can be added to assert success and
> > > > take a void pointer, but until then, forcing an out param yields ugly code
> > > > and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg().
> > > 
> > > This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the
> > > build on arm64:
> > > 
> > > aarch64/psci_test.c: In function ‘host_test_system_off2’:
> > > aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’
> > >   247 |         vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version);
> > >       |         ^~~~~~~~~~~~
> > > In file included from aarch64/psci_test.c:18:
> > > include/kvm_util.h:705:24: note: declared here
> > >   705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id)
> > >       |                        ^~~~~~~~~~~~
> > > At top level:
> > > cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at
> > > -end’ may have been intended to silence earlier diagnostics
> > > 
> > > since the updates done to that file did not take account of 72be5aa6be4
> > > ("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged
> > > in the kvm-arm64 tree.
> > 
> > Bugger.  In hindsight, it's obvious that of course arch selftests would add usage
> > of vcpu_get_reg().
> > 
> > Unless someone has a better idea, I'll drop the series from kvm-x86, post a new
> > version that applies on linux-next, and then re-apply the series just before the
> > v6.13 merge window (rinse and repeat as needed if more vcpu_get_reg() users come
> > along).
> 
> Can you instead just push out a topic branch and let the affected
> maintainers deal with it? This is the usual way we handle conflicts
> between trees...

That'd work too, but as you note below, doing that now throws a wrench in things
because essentially all arch maintainers would need merge that topic branch,
otherwise linux-next would end up in the same state.

> > That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g.
> > have a "selftests_late" or whatever topic branch.
> 
> The right time to do KVM-wide changes (even selftests) is *early* in the
> development cycle, not last minute. It gives us plenty of time to iron out
> the wrinkles.

Yeah, that was the original plan, then the stupid strict aliasing bug happened,
and I honestly forgot the vcpu_get_reg() changes would need to be consumed by
other architectures.

Other than letting me forget about this mess a few weeks earlier, there's no
good reason to force this into 6.13.  So, I'll drop the series from 6.13, post
new versions of the this and the $(ARCH) series just before the merge window,
and then either send a pull request to Paolo for 6.14 as soon as the 6.13 merge
window closes, or ask/bribe Paolo to apply everything directly.


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 15:59         ` Sean Christopherson
@ 2024-11-01 16:11           ` Oliver Upton
  2024-11-01 16:16             ` Sean Christopherson
  0 siblings, 1 reply; 28+ messages in thread
From: Oliver Upton @ 2024-11-01 16:11 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Mark Brown, Marc Zyngier, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

On Fri, Nov 01, 2024 at 08:59:16AM -0700, Sean Christopherson wrote:
> > Can you instead just push out a topic branch and let the affected
> > maintainers deal with it? This is the usual way we handle conflicts
> > between trees...
> 
> That'd work too, but as you note below, doing that now throws a wrench in things
> because essentially all arch maintainers would need merge that topic branch,
> otherwise linux-next would end up in the same state.

TBH, I'm quite happy with that. Recent history has not been particularly
convinincing to me that folks are actually testing arm64, let alone
compiling for it when applying selftests patches.

Otherwise, the alternative of respinning the global change for every -next
breakage adds a decent amount of toil and gives the wrong impression of how
long our patches have actually been baking in -next.

> > > That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g.
> > > have a "selftests_late" or whatever topic branch.
> > 
> > The right time to do KVM-wide changes (even selftests) is *early* in the
> > development cycle, not last minute. It gives us plenty of time to iron out
> > the wrinkles.
> 
> Yeah, that was the original plan, then the stupid strict aliasing bug happened,
> and I honestly forgot the vcpu_get_reg() changes would need to be consumed by
> other architectures.
> 
> Other than letting me forget about this mess a few weeks earlier, there's no
> good reason to force this into 6.13.  So, I'll drop the series from 6.13, post
> new versions of the this and the $(ARCH) series just before the merge window,
> and then either send a pull request to Paolo for 6.14 as soon as the 6.13 merge
> window closes, or ask/bribe Paolo to apply everything directly.

Sounds good, punting to 6.14 seems reasonable.

-- 
Thanks,
Oliver


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 16:11           ` Oliver Upton
@ 2024-11-01 16:16             ` Sean Christopherson
  2024-11-01 16:22               ` Oliver Upton
  0 siblings, 1 reply; 28+ messages in thread
From: Sean Christopherson @ 2024-11-01 16:16 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Mark Brown, Marc Zyngier, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

On Fri, Nov 01, 2024, Oliver Upton wrote:
> On Fri, Nov 01, 2024 at 08:59:16AM -0700, Sean Christopherson wrote:
> > > Can you instead just push out a topic branch and let the affected
> > > maintainers deal with it? This is the usual way we handle conflicts
> > > between trees...
> > 
> > That'd work too, but as you note below, doing that now throws a wrench in things
> > because essentially all arch maintainers would need merge that topic branch,
> > otherwise linux-next would end up in the same state.
> 
> TBH, I'm quite happy with that. Recent history has not been particularly
> convinincing to me that folks are actually testing arm64, let alone
> compiling for it when applying selftests patches.

FWIW, I did compile all patches on all KVM architectures, including selftests.
But my base obviously didn't include the kvm-arm64 branch :-/

One thing I'll add to my workflow would be to do a local merge (and smoke test)
of linux-next into kvm-x86 next before pushing it out.  This isn't the only snafu
this cycle where such a sanity check would have saved me and others a bit of pain.

https://lore.kernel.org/all/20241101153857.GAZyT2EdLXKs7ZmDFx@fat_crate.local


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 16:16             ` Sean Christopherson
@ 2024-11-01 16:22               ` Oliver Upton
  2024-11-01 16:31                 ` Sean Christopherson
  2024-11-01 16:37                 ` Mark Brown
  0 siblings, 2 replies; 28+ messages in thread
From: Oliver Upton @ 2024-11-01 16:22 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Mark Brown, Marc Zyngier, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

On Fri, Nov 01, 2024 at 09:16:42AM -0700, Sean Christopherson wrote:
> On Fri, Nov 01, 2024, Oliver Upton wrote:
> > On Fri, Nov 01, 2024 at 08:59:16AM -0700, Sean Christopherson wrote:
> > > > Can you instead just push out a topic branch and let the affected
> > > > maintainers deal with it? This is the usual way we handle conflicts
> > > > between trees...
> > > 
> > > That'd work too, but as you note below, doing that now throws a wrench in things
> > > because essentially all arch maintainers would need merge that topic branch,
> > > otherwise linux-next would end up in the same state.
> > 
> > TBH, I'm quite happy with that. Recent history has not been particularly
> > convinincing to me that folks are actually testing arm64, let alone
> > compiling for it when applying selftests patches.
> 
> FWIW, I did compile all patches on all KVM architectures, including selftests.
> But my base obviously didn't include the kvm-arm64 branch :-/

Oh, that rip wasn't aimed at you, commit 76f972c2cfdf ("KVM: selftests: Fix build
on architectures other than x86_64") just came to mind.

> One thing I'll add to my workflow would be to do a local merge (and smoke test)
> of linux-next into kvm-x86 next before pushing it out.  This isn't the only snafu
> this cycle where such a sanity check would have saved me and others a bit of pain.

Eh, shit happens, that's what -next is for :)

The only point I wanted to make was that it is perfectly fine by me to
spread the workload w/ a topic branch if things blow up sometime after
your changes show up in -next.

-- 
Thanks,
Oliver


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 16:22               ` Oliver Upton
@ 2024-11-01 16:31                 ` Sean Christopherson
  2024-11-01 16:37                 ` Mark Brown
  1 sibling, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-11-01 16:31 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Mark Brown, Marc Zyngier, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

On Fri, Nov 01, 2024, Oliver Upton wrote:
> On Fri, Nov 01, 2024 at 09:16:42AM -0700, Sean Christopherson wrote:
> > One thing I'll add to my workflow would be to do a local merge (and smoke test)
> > of linux-next into kvm-x86 next before pushing it out.  This isn't the only snafu
> > this cycle where such a sanity check would have saved me and others a bit of pain.
> 
> Eh, shit happens, that's what -next is for :)

Heh, but I also don't actually test -next, which was another snafu (not my fault
this time!) from this cycle[*].  Testing 6.12-next prior to the merge window
wouldn't have made that any less painful to bisect, but I think it would at least
have allowed me to detect that the issue specifically came in from linux-next,
and the bug report would have gotten to PeterZ almost two months earlier.

https://lore.kernel.org/all/ZwdA0sbA2tJA3IKh@google.com


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

* Re: [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
  2024-11-01 16:22               ` Oliver Upton
  2024-11-01 16:31                 ` Sean Christopherson
@ 2024-11-01 16:37                 ` Mark Brown
  1 sibling, 0 replies; 28+ messages in thread
From: Mark Brown @ 2024-11-01 16:37 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Sean Christopherson, Marc Zyngier, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, linux-arm-kernel, kvmarm, kvm,
	kvm-riscv, linux-riscv, linux-kernel, Andrew Jones,
	James Houghton, David Woodhouse, linux-next

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

On Fri, Nov 01, 2024 at 09:22:30AM -0700, Oliver Upton wrote:
> On Fri, Nov 01, 2024 at 09:16:42AM -0700, Sean Christopherson wrote:

> > One thing I'll add to my workflow would be to do a local merge (and smoke test)
> > of linux-next into kvm-x86 next before pushing it out.  This isn't the only snafu
> > this cycle where such a sanity check would have saved me and others a bit of pain.

> Eh, shit happens, that's what -next is for :)

> The only point I wanted to make was that it is perfectly fine by me to
> spread the workload w/ a topic branch if things blow up sometime after
> your changes show up in -next.

Yeah, the -next breakage is a bit annoying but so long as it gets fixed
promptly it's kind of what it's there for.  It's much more of an issue
when things make it into mainline, and can be very problematic if it
makes it into a tagged -rc (especially -rc1) or something.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress
  2024-10-31 19:51 ` [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
@ 2024-11-05  5:53   ` Sean Christopherson
  0 siblings, 0 replies; 28+ messages in thread
From: Sean Christopherson @ 2024-11-05  5:53 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Paolo Bonzini, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda
  Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
	linux-kernel, Andrew Jones, James Houghton

On Thu, Oct 31, 2024, Sean Christopherson wrote:
> On Wed, 09 Oct 2024 08:49:39 -0700, Sean Christopherson wrote:
> > The main purpose of this series is to convert the max_guest_memory_test
> > into a more generic mmu_stress_test.  The basic gist of the "conversion"
> > is to have the test do mprotect() on guest memory while vCPUs are
> > accessing said memory, e.g. to verify KVM and mmu_notifiers are working
> > as intended.
> > 
> > Patches 1-4 are a somewhat unexpected side quest.  The original plan was
> > that patch 3 would be a single patch, but things snowballed.
> > 
> > [...]
> 
> Applied to kvm-x86 selftests, with the typo fixup pointed out by James.  Thanks!
> 
> [01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header
>         https://github.com/kvm-x86/linux/commit/5e07fd0bf516
> [02/14] KVM: selftests: Disable strict aliasing
>         https://github.com/kvm-x86/linux/commit/d1ce2bcd8d2e
> [03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param
>         https://github.com/kvm-x86/linux/commit/5c6c7b71a45c
> [04/14] KVM: selftests: Assert that vcpu_{g,s}et_reg() won't truncate
>         https://github.com/kvm-x86/linux/commit/6aa2df3eb90b
> [05/14] KVM: selftests: Check for a potential unhandled exception iff KVM_RUN succeeded
>         https://github.com/kvm-x86/linux/commit/be9f2746d20b
> [06/14] KVM: selftests: Rename max_guest_memory_test to mmu_stress_test
>         https://github.com/kvm-x86/linux/commit/06694f27cfcc
> [07/14] KVM: selftests: Only muck with SREGS on x86 in mmu_stress_test
>         https://github.com/kvm-x86/linux/commit/8556ce365a07
> [08/14] KVM: selftests: Compute number of extra pages needed in mmu_stress_test
>         https://github.com/kvm-x86/linux/commit/c7b7876ac5d4
> [09/14] KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c
>         https://github.com/kvm-x86/linux/commit/a657856469e1
> [10/14] KVM: selftests: Enable mmu_stress_test on arm64
>         https://github.com/kvm-x86/linux/commit/1e53cde06102
> [11/14] KVM: selftests: Use vcpu_arch_put_guest() in mmu_stress_test
>         https://github.com/kvm-x86/linux/commit/8630563012b9
> [12/14] KVM: selftests: Precisely limit the number of guest loops in mmu_stress_test
>         https://github.com/kvm-x86/linux/commit/3d4585c220dc
> [13/14] KVM: selftests: Add a read-only mprotect() phase to mmu_stress_test
>         https://github.com/kvm-x86/linux/commit/eaafeebca75a
> [14/14] KVM: selftests: Verify KVM correctly handles mprotect(PROT_READ)
>         https://github.com/kvm-x86/linux/commit/a3cd5c187742

As mentioned later in the thread[*], I dropped this series from the 6.13 queue
and will instead target 6.14.

I did however grab the no-strict-aliasing fix for 6.12, and tagged it for stable.
There's no reason to wait to land that commit, and I definitely have no desire to
ever debug that mess again.

[02/14] KVM: selftests: Disable strict aliasing
      https://github.com/kvm-x86/linux/commit/5b188cc4866a

[*] https://lore.kernel.org/all/ZyT61FF0-g8gKZfc@google.com


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

end of thread, other threads:[~2024-11-05  5:55 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 15:49 [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 01/14] KVM: Move KVM_REG_SIZE() definition to common uAPI header Sean Christopherson
2024-10-10 11:41   ` Anup Patel
2024-10-09 15:49 ` [PATCH v3 02/14] KVM: selftests: Disable strict aliasing Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 03/14] KVM: selftests: Return a value from vcpu_get_reg() instead of using an out-param Sean Christopherson
2024-11-01 13:07   ` Mark Brown
2024-11-01 14:48     ` Sean Christopherson
2024-11-01 15:38       ` Oliver Upton
2024-11-01 15:59         ` Sean Christopherson
2024-11-01 16:11           ` Oliver Upton
2024-11-01 16:16             ` Sean Christopherson
2024-11-01 16:22               ` Oliver Upton
2024-11-01 16:31                 ` Sean Christopherson
2024-11-01 16:37                 ` Mark Brown
2024-10-09 15:49 ` [PATCH v3 04/14] KVM: selftests: Assert that vcpu_{g,s}et_reg() won't truncate Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 05/14] KVM: selftests: Check for a potential unhandled exception iff KVM_RUN succeeded Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 06/14] KVM: selftests: Rename max_guest_memory_test to mmu_stress_test Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 07/14] KVM: selftests: Only muck with SREGS on x86 in mmu_stress_test Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 08/14] KVM: selftests: Compute number of extra pages needed " Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 09/14] KVM: sefltests: Explicitly include ucall_common.h in mmu_stress_test.c Sean Christopherson
     [not found]   ` <CADrL8HUEwnP8e700y2XYDgVhhUJuj1UEJmd2NLdtZ1dV845DNw@mail.gmail.com>
2024-10-28 20:45     ` Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 10/14] KVM: selftests: Enable mmu_stress_test on arm64 Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 11/14] KVM: selftests: Use vcpu_arch_put_guest() in mmu_stress_test Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 12/14] KVM: selftests: Precisely limit the number of guest loops " Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 13/14] KVM: selftests: Add a read-only mprotect() phase to mmu_stress_test Sean Christopherson
2024-10-09 15:49 ` [PATCH v3 14/14] KVM: selftests: Verify KVM correctly handles mprotect(PROT_READ) Sean Christopherson
2024-10-31 19:51 ` [PATCH v3 00/14] KVM: selftests: Morph max_guest_mem to mmu_stress Sean Christopherson
2024-11-05  5:53   ` Sean Christopherson

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