Re: [RFC PATCH v1 1/2] selftests/x86: sysret_rip: Handle syscall in a FRED system

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
To: Andrew Cooper <Andrew.Cooper3@citrix.com>
Cc: Xin Li <xin3.li@intel.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Brian Gerst <brgerst@gmail.com>, Borislav Petkov <bp@alien8.de>,
	Shuah Khan <shuah@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	x86 Mailing List <x86@kernel.org>,
	Linux Kselftest Mailing List  <linux-kselftest@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v1 1/2] selftests/x86: sysret_rip: Handle syscall in a FRED system
Date: Wed, 15 Feb 2023 17:42:25 +0700	[thread overview]
Message-ID: <Y+y3EabhG5p3b8iZ@biznet-home.integral.gnuweeb.org> (raw)
In-Reply-To: <0b57d2ab-14e0-14a0-adf3-3186fd2f5282@citrix.com>

On Wed, Feb 15, 2023 at 09:17:23AM +0000, Andrew Cooper wrote:
> On 26/01/2023 8:08 pm, Ammar Faizi wrote:
> > What did I do wrong?
> 
> Well this is a fine mess...
> 
> https://godbolt.org/z/MaPM7s8qr does the right thing, but is now
> contrary to the prior discussion regarding calls in asm, which concluded
> that the "+r"(rsp) was the way to go.

Does that also mean the ASM_CALL_CONSTRAINT macro in
arch/x86/include/asm/asm.h macro is wrong?

That macro adds a "+r"(rsp) constraint, and we assume it's safe to
execute the "call" instruction with that constraint in an inline
Assembly.

I am not sure what "+r" (rsp) actually does. And if we are now
complaining, "+r" (rsp) doesn't work. Since when it works? Or at least,
where is that rule written?  I couldn't find any GCC or Clang version
that does it right with the "+r" (rsp) constraint (from a quick playing
with that godbolt link).

> Furthermore GCC regressed in 9.0 and emits:
> 
>   warning: listing the stack pointer register 'rsp' in a clobber list is
> deprecated [-Wdeprecated]
> 
> which might be the intention of the developers, but is wrong seeing as
> this is the only way to say "I modify the redzone" to the compiler...

Yeah, adding "rsp" to the clobber list works. But sadly, it's deprecated
in GCC. Not sure what the reason is.

I think the most straightforward and safest way, for now, is: "Don't
clobber the red zone from the inline asm.".

I will use the previous approach to avoid red-zone clobbering in the
next revision. That's by adding "r12" to the clobber list and preserving
the red zone content in "%r12". 

-- 
Ammar Faizi