| Message ID | 20230124100926.637335-1-ammarfaizi2@gnuweeb.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | selftests/x86: sysret_rip update for FRED system | expand |
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org> > > 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@intel.com > Fixes: 660602140103 ("selftests/x86: Add a selftest for SYSRET to noncanonical > addresses") > Link: https://lore.kernel.org/lkml/25b96960-a07e-a952-5c23- > 786b55054126@zytor.com > Reported-by: Xin Li <xin3.li@intel.com> > Co-developed-by: H. Peter Anvin (Intel) <hpa@zytor.com> > Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com> > Acked-by: H. Peter Anvin (Intel) <hpa@zytor.com> > Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org> > --- > > ## 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@zytor.com ) > > --- > > 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
On January 24, 2023 1:32:14 PM PST, "Li, Xin3" <xin3.li@intel.com> wrote: >> From: Ammar Faizi <ammarfaizi2@gnuweeb.org> >> >> 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@intel.com >> Fixes: 660602140103 ("selftests/x86: Add a selftest for SYSRET to noncanonical >> addresses") >> Link: https://lore.kernel.org/lkml/25b96960-a07e-a952-5c23- >> 786b55054126@zytor.com >> Reported-by: Xin Li <xin3.li@intel.com> >> Co-developed-by: H. Peter Anvin (Intel) <hpa@zytor.com> >> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com> >> Acked-by: H. Peter Anvin (Intel) <hpa@zytor.com> >> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org> >> --- >> >> ## 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@zytor.com ) >> >> --- >> >> 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.
On 24/01/2023 9:32 pm, Li, Xin3 wrote: >> From: Ammar Faizi <ammarfaizi2@gnuweeb.org> >> >> 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. Is this under SIMICS, or elsewhere? It's doubly weird that flags/%r11 match, but %rip/%rcx don't. ~Andrew
> >> > >> 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. > >> > > This should use check_regs_result() – which is exactly the reason I made that a > separate function. Exactly.
> >> assert(ctx->uc_mcontext.gregs[REG_EFL] == > >> ctx->uc_mcontext.gregs[REG_R11]); > > On FRED systems, flags is 0x200a93 and r11 is 0xfeedfacedeadbeef here. > > Is this under SIMICS, or elsewhere? It's doubly weird that flags/%r11 match, but > %rip/%rcx don't. This is on Intel Simics, with FRED enabled. Flags and %r11 don’t match on FRED, and %rip and %rcx also don't match. I think it's expected. Xin
On Tue, Jan 24, 2023 at 01:37:43PM -0800, H. Peter Anvin wrote: > On January 24, 2023 1:32:14 PM PST, "Li, Xin3" <xin3.li@intel.com> wrote: > >> From: Ammar Faizi <ammarfaizi2@gnuweeb.org> > >> > >> 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]); > > > > This should use check_regs_result() – which is exactly the reason I made that a separate function. Fixed in v4.