On January 24, 2023 1:32:14 PM PST, "Li, Xin3" <xin3.li@xxxxxxxxx> wrote: >> From: Ammar Faizi <ammarfaizi2@xxxxxxxxxxx> >> >> This is an RFC patchset v3: >> sysret_rip test update for Intel FRED architecture. >> >> Xin Li reported sysret_rip test fails at: >> >> assert(ctx->uc_mcontext.gregs[REG_EFL] == >> ctx->uc_mcontext.gregs[REG_R11]); > >On FRED systems, flags is 0x200a93 and r11 is 0xfeedfacedeadbeef here. > >We need to remove or change this assertion, maybe: > assert(ctx->uc_mcontext.gregs[REG_EFL] == ctx->uc_mcontext.gregs[REG_R11] || > r11_sentinel == ctx->uc_mcontext.gregs[REG_R11]); > >> >> in a FRED system. Let's handle the FRED system scenario too. The 'syscall' >> instruction in a FRED system doesn't set %r11=%rflags. >> >> There are two patches in this series. >> >> How to test this: >> >> $ make -C tools/testing/selftests/x86 >> $ tools/testing/selftests/x86/sysret_rip_64 >> >> Link: https://lore.kernel.org/lkml/5d4ad3e3-034f-c7da-d141- >> 9c001c2343af@xxxxxxxxx >> Fixes: 660602140103 ("selftests/x86: Add a selftest for SYSRET to noncanonical >> addresses") >> Link: https://lore.kernel.org/lkml/25b96960-a07e-a952-5c23- >> 786b55054126@xxxxxxxxx >> Reported-by: Xin Li <xin3.li@xxxxxxxxx> >> Co-developed-by: H. Peter Anvin (Intel) <hpa@xxxxxxxxx> >> Signed-off-by: H. Peter Anvin (Intel) <hpa@xxxxxxxxx> >> Acked-by: H. Peter Anvin (Intel) <hpa@xxxxxxxxx> >> Signed-off-by: Ammar Faizi <ammarfaizi2@xxxxxxxxxxx> >> --- >> >> ## Changelog v3: >> >> - Test that we don't get a mix of REGS_SAVED and REGS_SYSRET, >> which is a major part of the point (hpa). >> >> ## Changelog v2: >> >> - Use "+r"(rsp) as the right way to avoid redzone problems >> per Andrew's comment (hpa). >> (Ref: https://lore.kernel.org/lkml/8f5c24df-514d-5d89-f58f- >> ec8c3eb1e049@xxxxxxxxx ) >> >> --- >> >> Ammar Faizi (2): >> selftests/x86: sysret_rip: Handle syscall in a FRED system >> selftests/x86: sysret_rip: Add more syscall tests with respect to `%rcx` and `%r11` >> >> tools/testing/selftests/x86/sysret_rip.c | 120 ++++++++++++++++++++++- >> 1 file changed, 119 insertions(+), 1 deletion(-) >> >> >> base-commit: e12ad468c22065a2826b2fc4c11d2113a7975301 >> -- >> Ammar Faizi > > This should use check_regs_result() – which is exactly the reason I made that a separate function.