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

--UnkPqelfwiKSm0sl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

--UnkPqelfwiKSm0sl
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmpyK+sACgkQJNaLcl1U
h9BLFQf+M2EJkmtPKxPvAE1jYWYzT7FBTecWzsvhmXOcvfDOFM3DC9Mx06mhp3Xf
/8vRfb8xclnvXnepYxm43kzZCF8sfg7j7RHsumvyawkQe67rfgmKWr2XFuwgLIwi
Bs8qUQKvwH+6oN0HdRpCY7N2WXkvjmOHQeOb5AAM6QVnHIqk8xBaR4OpwCWeLmlF
IAk84krdI6Uns7W+2dhZwNFKNQGmV67595izCLGjPEZ79L1kNip1UfdaztYcgFPf
LczVv4t11khhG/KxYBnDIQHx4Fj0ph+iur2sNXP/ZRgt/wqEbSzj3FBr4pYMXWZm
pw+b7DT9DUuuBopgQmk/NbNAg096tA==
=SVXk
-----END PGP SIGNATURE-----

--UnkPqelfwiKSm0sl--