diff mbox series

[v10,36/38] x86/fred: Add fred_syscall_init()

Message ID 20230914044805.301390-37-xin3.li@intel.com (mailing list archive)
State Superseded
Headers show
Series x86: enable FRED for x86-64 | expand

Commit Message

Li, Xin3 Sept. 14, 2023, 4:48 a.m. UTC 
From: "H. Peter Anvin (Intel)" <hpa@zytor.com>

Add a syscall initialization function fred_syscall_init() for FRED,
and this is really just to skip setting up SYSCALL/SYSENTER related
MSRs, e.g., MSR_LSTAR and invalidate SYSENTER configurations, because
FRED uses the ring 3 FRED entrypoint for SYSCALL and SYSENTER, and
ERETU is the only legit instruction to return to ring 3 per FRED spec
5.0.

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Tested-by: Shan Kang <shan.kang@intel.com>
Signed-off-by: Xin Li <xin3.li@intel.com>
---
 arch/x86/kernel/cpu/common.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Thomas Gleixner Sept. 19, 2023, 8:28 a.m. UTC | #1 
On Wed, Sep 13 2023 at 21:48, Xin Li wrote:
> +static inline void fred_syscall_init(void)
> +{
> +	/*
> +	 * Per FRED spec 5.0, FRED uses the ring 3 FRED entrypoint for SYSCALL
> +	 * and SYSENTER, and ERETU is the only legit instruction to return to
> +	 * ring 3, as a result there is _no_ need to setup the SYSCALL and
> +	 * SYSENTER MSRs.
> +	 *
> +	 * Note, both sysexit and sysret cause #UD when FRED is enabled.
> +	 */
> +	wrmsrl(MSR_LSTAR, 0ULL);
> +	wrmsrl_cstar(0ULL);

That write is pointless. See the comment in wrmsrl_cstar().

Thanks,

        tglx
Li, Xin3 Sept. 20, 2023, 4:33 a.m. UTC | #2 
> > +static inline void fred_syscall_init(void) {
> > +	/*
> > +	 * Per FRED spec 5.0, FRED uses the ring 3 FRED entrypoint for SYSCALL
> > +	 * and SYSENTER, and ERETU is the only legit instruction to return to
> > +	 * ring 3, as a result there is _no_ need to setup the SYSCALL and
> > +	 * SYSENTER MSRs.
> > +	 *
> > +	 * Note, both sysexit and sysret cause #UD when FRED is enabled.
> > +	 */
> > +	wrmsrl(MSR_LSTAR, 0ULL);
> > +	wrmsrl_cstar(0ULL);
> 
> That write is pointless. See the comment in wrmsrl_cstar().

What I heard is that AMD is going to support FRED.

Both LSTAR and CSTAR have no function when FRED is enabled, so maybe
just do NOT write to them?

Thanks!
    Xin
Thomas Gleixner Sept. 20, 2023, 8:18 a.m. UTC | #3 
On Wed, Sep 20 2023 at 04:33, Li, Xin3 wrote:
>> > +static inline void fred_syscall_init(void) {
>> > +	/*
>> > +	 * Per FRED spec 5.0, FRED uses the ring 3 FRED entrypoint for SYSCALL
>> > +	 * and SYSENTER, and ERETU is the only legit instruction to return to
>> > +	 * ring 3, as a result there is _no_ need to setup the SYSCALL and
>> > +	 * SYSENTER MSRs.
>> > +	 *
>> > +	 * Note, both sysexit and sysret cause #UD when FRED is enabled.
>> > +	 */
>> > +	wrmsrl(MSR_LSTAR, 0ULL);
>> > +	wrmsrl_cstar(0ULL);
>> 
>> That write is pointless. See the comment in wrmsrl_cstar().
>
> What I heard is that AMD is going to support FRED.
>
> Both LSTAR and CSTAR have no function when FRED is enabled, so maybe
> just do NOT write to them?

Right. If AMD needs to clear it then it's trivial enough to add a
wrmsrl_cstar(0) to it.
H. Peter Anvin Sept. 21, 2023, 2:24 a.m. UTC | #4 
On September 20, 2023 1:18:14 AM PDT, Thomas Gleixner <tglx@linutronix.de> wrote:
>On Wed, Sep 20 2023 at 04:33, Li, Xin3 wrote:
>>> > +static inline void fred_syscall_init(void) {
>>> > +	/*
>>> > +	 * Per FRED spec 5.0, FRED uses the ring 3 FRED entrypoint for SYSCALL
>>> > +	 * and SYSENTER, and ERETU is the only legit instruction to return to
>>> > +	 * ring 3, as a result there is _no_ need to setup the SYSCALL and
>>> > +	 * SYSENTER MSRs.
>>> > +	 *
>>> > +	 * Note, both sysexit and sysret cause #UD when FRED is enabled.
>>> > +	 */
>>> > +	wrmsrl(MSR_LSTAR, 0ULL);
>>> > +	wrmsrl_cstar(0ULL);
>>> 
>>> That write is pointless. See the comment in wrmsrl_cstar().
>>
>> What I heard is that AMD is going to support FRED.
>>
>> Both LSTAR and CSTAR have no function when FRED is enabled, so maybe
>> just do NOT write to them?
>
>Right. If AMD needs to clear it then it's trivial enough to add a
>wrmsrl_cstar(0) to it.

Just to clarify: the only reason I added the writes here was to possibly make bugs easier to track down. There is indeed no functional reason.
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index d960b7276008..4cb36e241c9a 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2105,6 +2105,23 @@  static inline void idt_syscall_init(void)
 	       X86_EFLAGS_AC|X86_EFLAGS_ID);
 }
 
+static inline void fred_syscall_init(void)
+{
+	/*
+	 * Per FRED spec 5.0, FRED uses the ring 3 FRED entrypoint for SYSCALL
+	 * and SYSENTER, and ERETU is the only legit instruction to return to
+	 * ring 3, as a result there is _no_ need to setup the SYSCALL and
+	 * SYSENTER MSRs.
+	 *
+	 * Note, both sysexit and sysret cause #UD when FRED is enabled.
+	 */
+	wrmsrl(MSR_LSTAR, 0ULL);
+	wrmsrl_cstar(0ULL);
+	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
+	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
+	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
+}
+
 /* May not be marked __init: used by software suspend */
 void syscall_init(void)
 {