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

On 8/18/26 7:41 PM, [email protected] wrote:
> 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?

For any human to double check:

diff --git a/tools/testing/selftests/kvm/arm64/sea_to_user.c 
b/tools/testing/selftests/kvm/arm64/sea_to_user.c
index 7cf95da8e594d..f1d156d7645cc 100644
--- a/tools/testing/selftests/kvm/arm64/sea_to_user.c
+++ b/tools/testing/selftests/kvm/arm64/sea_to_user.c
@@ -165,11 +165,16 @@ static void guest_code(void)
  {
  	u64 guest_data;

-	/* Consumes error will cause a SEA. */
+	/* Consuming a real poison faults as an SEA and never returns here. */
  	guest_data = *(u64 *)EINJ_GVA;

-	GUEST_FAIL("Poison not protected by SEA: gva=%#lx, guest_data=%#lx\n",
-		   EINJ_GVA, guest_data);
+	/*
+	 * The read returned, so this platform placed no consumable poison.
+	 * Report the value read as a positive signal that is distinct from a
+	 * GUEST_ASSERT failure, so the host can skip on this limitation without
+	 * masking a genuine SEA-handling failure.
+	 */
+	GUEST_SYNC1(guest_data);
  }

  static void expect_sea_handler(struct ex_regs *regs)
@@ -215,13 +220,46 @@ 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 must trap to KVM as an
+	 * SEA. If the exit is something else, decode what the guest did before
+	 * deciding whether to skip or fail:
+	 *
+	 *  - UCALL_SYNC: the read returned instead of faulting, so no
+	 *    consumable poison was placed. Some firmware only arms EINJ poison
+	 *    as part of the trigger step that notrigger=1 skips, leaving
+	 *    nothing for the guest to consume; arm64 delivers the ucall as an
+	 *    MMIO write (hence KVM_EXIT_MMIO). This platform cannot host the
+	 *    test, so skip, matching the requirement documented at the top.
+	 *  - UCALL_ABORT: the guest took an abort but its SEA handler failed an
+	 *    assertion. That is a genuine problem, so report it rather than
+	 *    masking it as the limitation above.
+	 *  - anything else: fall through to the exit-reason assertion.
+	 */
+	if (run->exit_reason != KVM_EXIT_ARM_SEA) {
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_SYNC:
+			ksft_print_msg("Guest read back %#lx without an SEA\n",
+				       uc.args[0]);
+			ksft_exit_skip("EINJ notrigger placed no consumable poison on this 
platform\n");
+			break;
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+			break;
+		default:
+			break;
+		}
+	}
+
+	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",
  		       run->arm_sea.gva, run->arm_sea.gpa);

-	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_ARM_SEA);
-
  	esr = run->arm_sea.esr;
  	TEST_ASSERT_EQ(ESR_ELx_EC(esr), ESR_ELx_EC_DABT_LOW);
  	TEST_ASSERT_EQ(esr & ESR_ELx_FSC_TYPE, ESR_ELx_FSC_EXTABT);

> 
>> +		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",
>