Re: [PATCH v19 14/14] KVM: selftests: arm64: Add GCS EXLOCK exception emulation test

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Brittle test state causes confusing assertion failures on unexpected UNDEF exceptions.
--

--- Patch [14]: [PATCH v19 14/14] KVM: selftests: arm64: Add GCS EXLOCK exception emulation test ---
commit 3ce6c247a7bdb06355a23e53c25ee48d7bb16f9d
Author: Mark Brown <[email protected]>
KVM: selftests: arm64: Add GCS EXLOCK exception emulation test

This patch adds a KVM selftest to validate the emulation of GCSCR_EL2.EXLOCKEN
when running in vEL2. It uses nested UDFs to observe and modify PSTATE.EXLOCK,
testing both fast and slow paths.

> diff --git a/tools/testing/selftests/kvm/arm64/gcs_exceptions.c b/tools/testing/selftests/kvm/arm64/gcs_exceptions.c
> new file mode 100644
> index 0000000000000..de5bc65907272
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/arm64/gcs_exceptions.c

[ ... ]

> +static void guest_undef_handler(struct ex_regs *regs)
> +{

[ ... ]

> +	case UDF_SPSR_EL2:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				expect_exlock ? GCSCR_ELx_EXLOCKEN : 0);
> +
> +		asm volatile("mrs %0, spsr_el2   \n\
> +			      msr spsr_el2, %0"
> +			     : "=r"(val) :: "memory");
> +		break;
> +
> +	default:
> +		GUEST_FAIL("Invalid udf_mode");
> +	}
> +}

[ ... ]

> +static void guest_code(void)
> +{

[ ... ]

> +	test_udf_exception(true);
> +	test_sysreg(UDF_ELR_EL1, true);
> +	test_sysreg(UDF_ELR_EL2, true);
> +	test_sysreg(UDF_SPSR_EL1, true);
> +	test_sysreg(UDF_SPSR_EL2, true);
> +
> +	/* Force ERET directly through the slow emulation path as well. */
> +	sysreg_clear_set_s(SYS_HFGITR_EL2, 0, HFGITR_EL2_ERET);

[Severity: Medium]
What happens if the vCPU does not support Fine Grained Traps (FEAT_FGT) when
this register is accessed?

Since this access occurs without verifying FGT support, it seems it would
trigger an unexpected UNDEFINED exception on systems where FGT is missing or
hidden.

Because udf_mode is not reset after the last intentional udf and remains set to
UDF_SPSR_EL2, would this unexpected exception incorrectly fall into the
UDF_SPSR_EL2 case in guest_undef_handler(), resulting in a confusing assertion
failure rather than cleanly reporting the unexpected UNDEF?

> +	isb();
> +
> +	test_udf_exception(false);
> +
> +	test_udf_exception(true);
> +
> +	sysreg_clear_set_s(SYS_HFGITR_EL2, HFGITR_EL2_ERET, 0);
> +	isb();
> +	GUEST_DONE();
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14