Re: [PATCH v2 3/3] KVM: selftests: arm64: Skip sea_to_user when EINJ places no poison

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The check skips the test unconditionally on any guest assertion failure, masking unrelated or future bugs.
--

--- Patch [3]: [PATCH v2 3/3] KVM: selftests: arm64: Skip sea_to_user when EINJ places no poison ---
commit 3bedaa634d562c6cc85fb3726d30fa40f0ed350d
Author: Like Xu <[email protected]>

KVM: selftests: arm64: Skip sea_to_user when EINJ places no poison

This commit modifies the sea_to_user selftest to skip rather than fail
when the firmware does not leave consumable poison in memory. It detects
this condition by checking if the exit reason is not KVM_EXIT_ARM_SEA
and a UCALL_ABORT is received from the guest.

> diff --git a/tools/testing/selftests/kvm/arm64/sea_to_user.c b/tools/testing/selftests/kvm/arm64/sea_to_user.c
> index 7cf95da8e594d..d4af6e0eed288 100644
> --- a/tools/testing/selftests/kvm/arm64/sea_to_user.c
> +++ b/tools/testing/selftests/kvm/arm64/sea_to_user.c
> @@ -215,13 +215,33 @@ static void run_vm(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
>  
>  	ksft_print_msg("Dump kvm_run info about KVM_EXIT_%s\n",
>  		       exit_reason_str(run->exit_reason));
> +
> +	/*
> +	 * The guest's read of the injected location is expected to trap to KVM
> +	 * as an SEA. If it does not, the injected error was never placed as
> +	 * consumable poison: some firmware honours EINJ's notrigger request by
> +	 * arming the poison only as part of the (now skipped) trigger step, so
> +	 * nothing is left in memory for the guest to consume. The guest then
> +	 * reads back the sentinel value and reports it via GUEST_FAIL, which
> +	 * arm64 delivers as a ucall over MMIO (hence a KVM_EXIT_MMIO here).
> +	 * Treat that as "this platform cannot host the test" and skip, matching
> +	 * the requirement documented at the top of this file, rather than
> +	 * failing on a hardware/firmware limitation the test cannot control.
> +	 */
> +	if (run->exit_reason != KVM_EXIT_ARM_SEA &&
> +	    get_ucall(vcpu, &uc) == UCALL_ABORT) {

[Severity: Medium]
Does this unconditionally skip the test on any guest assertion failure,
masking unrelated or future bugs?

If KVM fails to exit to userspace and instead incorrectly injects a
Data Abort to the guest with an unexpected syndrome, the guest's SEA
handler might fail a GUEST_ASSERT_EQ check. This would emit a
UCALL_ABORT over MMIO.

The host loop here interprets any UCALL_ABORT during this phase as a
missing EINJ poison limitation, skipping the test instead of reporting
the failure.

Can we check the specific assertion failure reason or uc.buffer string
instead of catching all UCALL_ABORTs?

> +		ksft_print_msg("Guest consumed no SEA: %s", uc.buffer);
> +		ksft_exit_skip("EINJ notrigger placed no consumable poison on this platform\n");
> +	}
> +
> +	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_ARM_SEA);
> +
> +	/* arm_sea holds valid data only for a KVM_EXIT_ARM_SEA exit. */
>  	ksft_print_msg("kvm_run.arm_sea: esr=%#llx, flags=%#llx\n",
>  		       run->arm_sea.esr, run->arm_sea.flags);
>  	ksft_print_msg("kvm_run.arm_sea: gva=%#llx, gpa=%#llx\n",

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