Re: [PATCH v1] KVM: selftests: Improve sea_to_user test
Like Xu <[email protected]>
| Newsgroups | dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest |
|---|---|
| Message-ID | <[email protected]> |
On 1/31/26 8:37 PM, Marc Zyngier wrote: > On Fri, 30 Jan 2026 19:28:37 +0000, > Jiaqi Yan <[email protected]> wrote: >> >> Several improvments to the test for KVM_EXIT_ARM_SEA: >> >> - Refactor run_vm to catch GUEST_FAIL, instead of causing confusing >> unhandled MMIO kvm exit. >> >> - Sync far_invalid to guest. >> >> - Exit test with KSFT_SKIP or KSFT_FAIL when should. >> >> - Add comment about VM backing memory type. >> >> Signed-off-by: Jiaqi Yan <[email protected]> >> --- >> .../testing/selftests/kvm/arm64/sea_to_user.c | 94 +++++++++++-------- >> 1 file changed, 53 insertions(+), 41 deletions(-) > > Overall, this test is still pretty broken. > > For example, on my Altra box: > > maz@filthy-habits:~$ ./sea_to_user > Random seed: 0x6b8b4567 > # Mapped 0x40000 pages: gva=0x80000000 to gpa=0xff80000000 > # Before EINJect: data=0xbaadcafe > # EINJ_GVA=0x81234bad, einj_gpa=0xff81234bad, einj_hva=0xffff41234bad, einj_hpa=0xbad > ok 1 # SKIP EINJ table not available in firmware > > Well, not quite. EINJ *is* available, it is just that this test, > contrary to *all* the other tests, requires some insanely high > privileges. But the test is making stupid assumption. > > But that's not all: > > maz@filthy-habits:~$ sudo ./sea_to_user > Random seed: 0x6b8b4567 > # Mapped 0x40000 pages: gva=0x80000000 to gpa=0xff80000000 > # Before EINJect: data=0xbaadcafe > # EINJ_GVA=0x81234bad, einj_gpa=0xff81234bad, einj_hva=0xffff41234bad, einj_hpa=0x80041234bad > # echo 0x10 > /sys/kernel/debug/apei/einj/error_type - done > # echo 0x2 > /sys/kernel/debug/apei/einj/flags - done > # echo 0x80041234bad > /sys/kernel/debug/apei/einj/param1 - done > # echo 0xffffffffffffffff > /sys/kernel/debug/apei/einj/param2 - done > # echo 0x1 > /sys/kernel/debug/apei/einj/notrigger - done > sh: 1: echo: echo: I/O error > Bail out! Failed to write EINJ entry: Success (0) > # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Failed? Or Success? Who knows... > > But frankly, the whole "embedded shell script" is disgusting. What is > wrong with driving the injection from the test itself, rather than > this access()+popen()? popen() itself is a liability (see the man page > for system()), and should never be used for this sort of things. > >> >> diff --git a/tools/testing/selftests/kvm/arm64/sea_to_user.c b/tools/testing/selftests/kvm/arm64/sea_to_user.c >> index 573dd790aeb8e..4a3511fa1f940 100644 >> --- a/tools/testing/selftests/kvm/arm64/sea_to_user.c >> +++ b/tools/testing/selftests/kvm/arm64/sea_to_user.c >> @@ -12,6 +12,11 @@ >> * including the notrigger feature. Otherwise the test will be skipped. >> * The under-test platform's APEI should be unable to claim SEA. Otherwise >> * the test will also be skipped. >> + * >> + * The VM backing memory is tied to HugeTLB 1G hugepage so far. Make sure >> + * there are more than 4 1G hugepage on the system. They can be allocated > > Why *more than*? Isn't that *at least* instead? > >> + * at runtime by: >> + * echo 4 > /sys/kernel/mm/hugepages/hugepages-1048576kB/nr_hugepages >> */ > > Why can't you provide an adequate diagnostic instead of this stuff: > > maz@filthy-habits:~$ ./sea_to_user > Random seed: 0x6b8b4567 > ==== Test Assertion Failure ==== > include/kvm_syscalls.h:58: mem != MAP_FAILED > pid=887 tid=887 errno=12 - Cannot allocate memory > 1 0x0000000000405b4f: __kvm_mmap at kvm_syscalls.h:58 (discriminator 3) > 2 (inlined by) kvm_mmap at kvm_syscalls.h:65 (discriminator 3) > 3 (inlined by) vm_mem_add at kvm_util.c:1036 (discriminator 3) > 4 0x0000000000402373: vm_create_with_sea_handler at sea_to_user.c:290 > 5 (inlined by) main at sea_to_user.c:336 > 6 0x0000ffffb0dc229b: ?? ??:0 > 7 0x0000ffffb0dc237b: ?? ??:0 > 8 0x00000000004027ef: _start at ??:? > mmap() failed, rc: -1 errno: 12 (Cannot allocate memory) > > What is preventing you from not requiring huge pages? And not > requiring 4kB as the base page size? > > Given how broken this is, I'm likely to disable this test until you > fix it for real. Pls help review the expect output: # selftests: kvm: sea_to_user # Random seed: 0x6b8b4567 # # Mapped 0x40000 pages: gva=0x80000000 to gpa=0xff80000000 # # Before EINJect: data=0xbaadcafe # # EINJ_GVA=0x81234bad, einj_gpa=0xff81234bad, einj_hva=0xffff41234bad, einj_hpa=0x2141234bad # 1..0 # SKIP EINJ debugfs interface not available; run as root with CONFIG_ACPI_APEI_EINJ and firmware EINJ support ok 1 selftests: kvm: sea_to_user # SKIP # 1 skipped test(s) detected. Consider enabling relevant config options to improve coverage. # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:1 error:0 , and the diff behind: diff --git a/tools/testing/selftests/kvm/arm64/sea_to_user.c b/tools/testing/selftests/kvm/arm64/sea_to_user.c index a23d74ed45035..7e86cbeb55291 100644 --- a/tools/testing/selftests/kvm/arm64/sea_to_user.c +++ b/tools/testing/selftests/kvm/arm64/sea_to_user.c @@ -86,25 +86,32 @@ static u64 translate_hva_to_hpa(unsigned long hva) static void write_einj_entry(const char *einj_path, u64 val) { - char cmd[256] = {0}; - FILE *cmdfile = NULL; + char buf[32]; + int fd, len, ret; - sprintf(cmd, "echo %#lx > %s", val, einj_path); - cmdfile = popen(cmd, "r"); + fd = open(einj_path, O_WRONLY); + if (fd < 0) + ksft_exit_fail_perror(einj_path); - if (pclose(cmdfile) == 0) - ksft_print_msg("echo %#lx > %s - done\n", val, einj_path); - else - ksft_exit_fail_perror("Failed to write EINJ entry"); + len = snprintf(buf, sizeof(buf), "%#lx\n", val); + ret = write(fd, buf, len); + if (ret != len) + ksft_exit_fail_perror(einj_path); + + close(fd); + ksft_print_msg("%#lx > %s - done\n", val, einj_path); } static void inject_uer(u64 hpa) { - if (access("/sys/firmware/acpi/tables/EINJ", R_OK) == -1) - ksft_test_result_skip("EINJ table no available in firmware"); - + /* + * Reading the ACPI EINJ table requires CAP_SYS_ADMIN, so a plain + * access() check gives a false negative for unprivileged runs. Probe + * the debugfs EINJ interface instead: it is only present when both + * firmware and the kernel EINJ driver support error injection. + */ if (access(EINJ_ETYPE, R_OK | W_OK) == -1) - ksft_test_result_skip("EINJ module probably not loaded?"); + ksft_exit_skip("EINJ debugfs interface not available; run as root with CONFIG_ACPI_APEI_EINJ and firmware EINJ support\n"); write_einj_entry(EINJ_ETYPE, ERROR_TYPE_MEMORY_UER); write_einj_entry(EINJ_FLAGS, MASK_MEMORY_UER); > > Thanks, > > M. >