diff mbox series

[RFC,v2,2/2] selftests/x86: sysret_rip: Add more syscall tests with respect to `%rcx` and `%r11`

Message ID 20230124022729.596997-3-ammarfaizi2@gnuweeb.org
State Superseded
Headers show
Series None | expand

Commit Message

Ammar Faizi Jan. 24, 2023, 2:27 a.m. UTC 
From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Test that:

 - "syscall" in a FRED system doesn't clobber %rcx and %r11.
 - "syscall" in a non-FRED system sets %rcx=%rip and %r11=%rflags.

Test them out with a trivial system call like __NR_getppid and friends
which are extremely likely to return with SYSRET on an IDT system; check
that it returns a nonnegative value and then save the result.

Link: https://lore.kernel.org/lkml/25b96960-a07e-a952-5c23-786b55054126@zytor.com
Co-developed-by: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Need hpa's sign off.

 tools/testing/selftests/x86/sysret_rip.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Li, Xin3 Jan. 25, 2023, 5:07 p.m. UTC | #1 
> > This version passes on FRED, thanks a lot for quickly fixing it.
> 
> Great!
> 
> Can you pick these two patches and include it in the next version of
> "x86: enable FRED for x86-64" RFC patchset?

Would it be better to get this patch set merged first?

Otherwise surely I will include it in the FRED patch set.

  Xin
H. Peter Anvin Jan. 25, 2023, 5:24 p.m. UTC | #2 
On January 25, 2023 9:07:18 AM PST, "Li, Xin3" <xin3.li@intel.com> wrote:
>> > This version passes on FRED, thanks a lot for quickly fixing it.
>> 
>> Great!
>> 
>> Can you pick these two patches and include it in the next version of
>> "x86: enable FRED for x86-64" RFC patchset?
>
>Would it be better to get this patch set merged first?
>
>Otherwise surely I will include it in the FRED patch set.
>
>  Xin
>
> 
>

If the maintainers are ok with it, it would be better to merge it sooner: once we have agreed on the semantics, which I believe we have, we should be testing those semantics and nothing else.
Ammar Faizi Jan. 25, 2023, 5:41 p.m. UTC | #3 
On Wed, Jan 25, 2023 at 09:24:40AM -0800, H. Peter Anvin wrote:
> On January 25, 2023 9:07:18 AM PST, "Li, Xin3" <xin3.li@intel.com> wrote:
> > Would it be better to get this patch set merged first?
> > 
> > Otherwise surely I will include it in the FRED patch set.
> 
> If the maintainers are ok with it, it would be better to merge it
> sooner: once we have agreed on the semantics, which I believe we
> have, we should be testing those semantics and nothing else.

OK, let's keep this patchset separated from the FRED support
patchset.

In the meantime, let me address the recent HPA's comments.
Ammar Faizi Feb. 18, 2023, 4:27 a.m. UTC | #4 
On Wed, Feb 15, 2023 at 07:42:47AM +0000, Li, Xin3 wrote:
> Hi Faizi,
> 
> Any update on this patch set?

No comment from HPA. But after the recent discussion with Andrew, I
think at least it's now clear that we are not going to use "+r"(rsp) to
avoid the red zone problem.

I am on leave today. Will send revision v8 on Monday.

Thanks,
H. Peter Anvin Feb. 18, 2023, 4:51 a.m. UTC | #5 
On February 17, 2023 8:27:40 PM PST, Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
>On Wed, Feb 15, 2023 at 07:42:47AM +0000, Li, Xin3 wrote:
>> Hi Faizi,
>> 
>> Any update on this patch set?
>
>No comment from HPA. But after the recent discussion with Andrew, I
>think at least it's now clear that we are not going to use "+r"(rsp) to
>avoid the red zone problem.
>
>I am on leave today. Will send revision v8 on Monday.
>
>Thanks,
>

My apologies, I missed your latest response in the torrent of email. The redzone issue is weird; it ought to be breaking all over the place, not just this.

Let me take a quick look at it...
diff mbox series

Patch

diff --git a/tools/testing/selftests/x86/sysret_rip.c b/tools/testing/selftests/x86/sysret_rip.c
index 550bc4e69ac46997..75c72d34dbc5840c 100644
--- a/tools/testing/selftests/x86/sysret_rip.c
+++ b/tools/testing/selftests/x86/sysret_rip.c
@@ -252,8 +252,17 @@  static void test_syscall_fallthrough_to(unsigned long ip)
 	printf("[OK]\tWe survived\n");
 }
 
+static void test_syscall_rcx_r11(void)
+{
+	do_syscall(__NR_getpid, 0, 0, 0, 0, 0, 0);
+	do_syscall(__NR_gettid, 0, 0, 0, 0, 0, 0);
+	do_syscall(__NR_getppid, 0, 0, 0, 0, 0, 0);
+}
+
 int main()
 {
+	test_syscall_rcx_r11();
+
 	/*
 	 * When the kernel returns from a slow-path syscall, it will
 	 * detect whether SYSRET is appropriate.  If it incorrectly