[PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

[PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

[Fuad Tabba]

Add a set_id_regs case covering ID register immutability when a vCPU's
first KVM_RUN fails after finalization but before
KVM_ARCH_FLAG_HAS_RAN_ONCE is set. The test provokes such a failure with
a PMUv3-enabled vCPU whose PMU is left uninitialized, then checks that
KVM_SET_ONE_REG on an ID register and KVM_CREATE_DEVICE for a vGIC are
both rejected with -EBUSY.

Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---

Notes:
    This exercises the bug fixed by the KVM/arm64 ID register finalisation
    series and fails without it, so it should be applied on top of that
    series:
    
    https://lore.kernel.org/r/20260803-kvm-arm64-idreg-final-v2-0-d7d7e4efc640@kernel.org

 .../testing/selftests/kvm/arm64/set_id_regs.c | 92 ++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 7429a1055df56..f153d56b6e7f0 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -13,6 +13,7 @@
 #include "kvm_util.h"
 #include "processor.h"
 #include "test_util.h"
+#include "vgic.h"
 #include <linux/bitfield.h>
 
 enum ftr_type {
@@ -803,6 +804,93 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
 	ksft_test_result_pass("%s\n", __func__);
 }
 
+/*
+ * ID registers must stay immutable even when a vCPU's first KVM_RUN fails
+ * after finalization but before KVM_ARCH_FLAG_HAS_RAN_ONCE is set.
+ */
+static void test_idreg_frozen_after_failed_run(void)
+{
+	u64 reg = 0, val = 0, new_val = 0;
+	struct kvm_vcpu_init init;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	bool found = false;
+	int r;
+
+	if (!kvm_has_cap(KVM_CAP_ARM_PMU_V3)) {
+		ksft_test_result_skip("%s (PMUv3 unsupported)\n", __func__);
+		return;
+	}
+
+	/*
+	 * Fail the first run after finalization: a PMUv3 vCPU left without PMU
+	 * init is rejected by kvm_arm_pmu_v3_enable(). Skip the default vGIC so
+	 * the KVM_CREATE_DEVICE gate can also be exercised.
+	 */
+	test_disable_default_vgic();
+
+	vm = vm_create(1);
+	kvm_get_default_vcpu_target(vm, &init);
+	init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
+	vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
+	kvm_arch_vm_finalize_vcpus(vm);
+
+	r = _vcpu_run(vcpu);
+	TEST_ASSERT(r < 0 && errno == EINVAL,
+		    "first KVM_RUN should fail post-finalization: r=%d errno=%d",
+		    r, errno);
+
+	/* Pick a field whose lower value would be accepted before finalization. */
+	for (int i = 0; i < ARRAY_SIZE(test_regs) && !found; i++) {
+		const struct reg_ftr_bits *ftr_bits = test_regs[i].ftr_bits;
+
+		reg = KVM_ARM64_SYS_REG(test_regs[i].reg);
+		val = vcpu_get_reg(vcpu, reg);
+
+		for (int j = 0; ftr_bits[j].type != FTR_END; j++) {
+			u64 ftr = (val & ftr_bits[j].mask) >> ftr_bits[j].shift;
+			u64 safe = get_safe_value(&ftr_bits[j], ftr);
+
+			/* Skip fields KVM re-derives itself, e.g. the GIC field. */
+			if (ftr_bits[j].mutable || safe == ftr)
+				continue;
+
+			new_val = (val & ~ftr_bits[j].mask) |
+				  (safe << ftr_bits[j].shift);
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		ksft_test_result_skip("%s (no immutable ID reg field to test)\n",
+				      __func__);
+		kvm_vm_free(vm);
+		return;
+	}
+
+	r = __vcpu_set_reg(vcpu, reg, new_val);
+	TEST_ASSERT(r < 0 && errno == EBUSY,
+		    "ID reg write after failed first run: r=%d errno=%d",
+		    r, errno);
+	TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);
+
+	/* A write matching the finalized value is still accepted. */
+	r = __vcpu_set_reg(vcpu, reg, val);
+	TEST_ASSERT(!r, "matching ID reg write should be accepted: r=%d", r);
+
+	/* Creating an in-kernel irqchip after finalization must be rejected. */
+	if (kvm_supports_vgic_v3()) {
+		r = __kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3);
+		TEST_ASSERT(r < 0 && errno == EBUSY,
+			    "vGIC creation after failed first run: r=%d errno=%d",
+			    r, errno);
+	}
+
+	kvm_vm_free(vm);
+	ksft_test_result_pass("%s\n", __func__);
+}
+
 int main(void)
 {
 	struct kvm_vcpu *vcpu;
@@ -828,7 +916,7 @@ int main(void)
 
 	ksft_print_header();
 
-	test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
+	test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
 	for (i = 0; i < ARRAY_SIZE(test_regs); i++)
 		for (j = 0; test_regs[i].ftr_bits[j].type != FTR_END; j++)
 			test_cnt++;
@@ -847,5 +935,7 @@ int main(void)
 
 	kvm_vm_free(vm);
 
+	test_idreg_frozen_after_failed_run();
+
 	ksft_finished();
 }
-- 
2.39.5

Re: [PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

[Mark Brown]

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

On Tue, Aug 04, 2026 at 10:24:21AM +0100, Fuad Tabba wrote:
> Add a set_id_regs case covering ID register immutability when a vCPU's
> first KVM_RUN fails after finalization but before
> KVM_ARCH_FLAG_HAS_RAN_ONCE is set. The test provokes such a failure with
> a PMUv3-enabled vCPU whose PMU is left uninitialized, then checks that
> KVM_SET_ONE_REG on an ID register and KVM_CREATE_DEVICE for a vGIC are
> both rejected with -EBUSY.

> +	for (int i = 0; i < ARRAY_SIZE(test_regs) && !found; i++) {
> +		const struct reg_ftr_bits *ftr_bits = test_regs[i].ftr_bits;
> +
> +		reg = KVM_ARM64_SYS_REG(test_regs[i].reg);
> +		val = vcpu_get_reg(vcpu, reg);

If we're doing a focused test for the specific issue there's the VM ID
registers as well as the general ID registers and adding an irqchip.

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

Re: [PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

[Fuad Tabba]

On Tue, 4 Aug 2026 at 17:14, Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Aug 04, 2026 at 10:24:21AM +0100, Fuad Tabba wrote:
> > Add a set_id_regs case covering ID register immutability when a vCPU's
> > first KVM_RUN fails after finalization but before
> > KVM_ARCH_FLAG_HAS_RAN_ONCE is set. The test provokes such a failure with
> > a PMUv3-enabled vCPU whose PMU is left uninitialized, then checks that
> > KVM_SET_ONE_REG on an ID register and KVM_CREATE_DEVICE for a vGIC are
> > both rejected with -EBUSY.
>
> > +     for (int i = 0; i < ARRAY_SIZE(test_regs) && !found; i++) {
> > +             const struct reg_ftr_bits *ftr_bits = test_regs[i].ftr_bits;
> > +
> > +             reg = KVM_ARM64_SYS_REG(test_regs[i].reg);
> > +             val = vcpu_get_reg(vcpu, reg);
>
> If we're doing a focused test for the specific issue there's the VM ID
> registers as well as the general ID registers and adding an irqchip.

Sure, I'll expand it and send something out tomorrow.

Cheers,
/fuad

Re: [PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

[Mark Brown]

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

On Tue, Aug 04, 2026 at 10:24:21AM +0100, Fuad Tabba wrote:
> Add a set_id_regs case covering ID register immutability when a vCPU's
> first KVM_RUN fails after finalization but before

A couple of other things - there's plenty of other examples of these
problems in set_id_regs, I've got a series which I think mostly cleans
it up:

   https://patch.msgid.link/20260719-kvm-arm64-set-id-regs-aarch64-v6-0-724287f5f108@kernel.org

but probably best not to make the issue worse.

> +	if (!found) {
> +		ksft_test_result_skip("%s (no immutable ID reg field to test)\n",
> +				      __func__);
> +		kvm_vm_free(vm);
> +		return;
> +	}

All test result reports for a given test should use the same string when
reporting so that automation can figure out that results for a given
test from different runs correspond to each other.  It's better to print
a diagnostic message, then the ksft_test_result_() with the result for
the framework.

> +	TEST_ASSERT(r < 0 && errno == EBUSY,
> +		    "ID reg write after failed first run: r=%d errno=%d",
> +		    r, errno);
> +	TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);

> +	kvm_vm_free(vm);
> +	ksft_test_result_pass("%s\n", __func__);

Mixing TEST_ASSERT() and ksft_test_result_ in the same test program also
interacts really poorly with automation, TEST_ASSERT() just kills the
entire program so no result is reported for the actual failing test and
any tests the program would attempt to run afterwards also vanish from
the results.  If the test program is using the kselftest framework to
report tests as this one is it should report the results via kselftest
framework.  A better pattern would be something like:

	if (r < 0 && errno == EBUSY) {
		ksft_print_msg("ID reg write after failed first run: r=%d errno=%d",
				r, errno);
		pass = false;
		goto out;
	}

...

 out:
	kvm_vm_free(vm);
	ksft_test_result(pass, "%s\n", __func__);

TEST_ASSERT() is a good fit for tests that are just one test case per
program, then it plays nicely with tooling since the tooling just
tracking the executable result and any output is diagnostic logging.

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

Re: [PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

[Fuad Tabba]

On Tue, 4 Aug 2026 at 19:14, Mark Brown <broonie@kernel.org> wrote:
...
> > +     if (!found) {
> > +             ksft_test_result_skip("%s (no immutable ID reg field to test)\n",
> > +                                   __func__);
> > +             kvm_vm_free(vm);
> > +             return;
> > +     }
>
> All test result reports for a given test should use the same string when
> reporting so that automation can figure out that results for a given
> test from different runs correspond to each other.  It's better to print
> a diagnostic message, then the ksft_test_result_() with the result for
> the framework.

Thanks, will fix in v2: the skip will report under the same name, with
the reason as a ksft_print_msg().

>
> > +     TEST_ASSERT(r < 0 && errno == EBUSY,
> > +                 "ID reg write after failed first run: r=%d errno=%d",
> > +                 r, errno);
> > +     TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);
>
> > +     kvm_vm_free(vm);
> > +     ksft_test_result_pass("%s\n", __func__);
>
> Mixing TEST_ASSERT() and ksft_test_result_ in the same test program also
> interacts really poorly with automation, TEST_ASSERT() just kills the

Agreed, but it looks file-wide rather than specific to this test, so I
would rather leave it to your series than convert one function. Happy
to follow once that lands.

Cheers,
/fuad


> entire program so no result is reported for the actual failing test and
> any tests the program would attempt to run afterwards also vanish from
> the results.  If the test program is using the kselftest framework to
> report tests as this one is it should report the results via kselftest
> framework.  A better pattern would be something like:
>
>         if (r < 0 && errno == EBUSY) {
>                 ksft_print_msg("ID reg write after failed first run: r=%d errno=%d",
>                                 r, errno);
>                 pass = false;
>                 goto out;
>         }
>
> ...
>
>  out:
>         kvm_vm_free(vm);
>         ksft_test_result(pass, "%s\n", __func__);
>
> TEST_ASSERT() is a good fit for tests that are just one test case per
> program, then it plays nicely with tooling since the tooling just
> tracking the executable result and any output is diagnostic logging.

Re: [PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

[Mark Brown]

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

On Tue, Aug 04, 2026 at 07:30:44PM +0100, Fuad Tabba wrote:
> On Tue, 4 Aug 2026 at 19:14, Mark Brown <broonie@kernel.org> wrote:

> > > +     TEST_ASSERT(r < 0 && errno == EBUSY,
> > > +                 "ID reg write after failed first run: r=%d errno=%d",
> > > +                 r, errno);
> > > +     TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);

> > > +     kvm_vm_free(vm);
> > > +     ksft_test_result_pass("%s\n", __func__);

> > Mixing TEST_ASSERT() and ksft_test_result_ in the same test program also
> > interacts really poorly with automation, TEST_ASSERT() just kills the

> Agreed, but it looks file-wide rather than specific to this test, so I
> would rather leave it to your series than convert one function. Happy
> to follow once that lands.

Unfortunately at the minute it's not even consistent within the program -
the MPAM tests in there do use ksft_ based reporting, while they do so
with inconsistent strings you at least get all the results out if it's
just them that blow up.

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