| Message ID | 20231003062458.23552-20-xin3.li@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: enable FRED for x86-64 | expand |
On Mon, Oct 02, 2023 at 11:24:40PM -0700, Xin Li wrote: > From: "H. Peter Anvin (Intel)" <hpa@zytor.com> > > MSR_IA32_FRED_RSP0 is used during ring 3 event delivery, and needs to > be updated to point to the top of next task stack during task switch. > > 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/include/asm/switch_to.h | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h > index f42dbf17f52b..c3bd0c0758c9 100644 > --- a/arch/x86/include/asm/switch_to.h > +++ b/arch/x86/include/asm/switch_to.h > @@ -70,9 +70,13 @@ static inline void update_task_stack(struct task_struct *task) > #ifdef CONFIG_X86_32 > this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0); > #else > - /* Xen PV enters the kernel on the thread stack. */ > - if (cpu_feature_enabled(X86_FEATURE_XENPV)) > + if (cpu_feature_enabled(X86_FEATURE_FRED)) { > + /* WRMSRNS is a baseline feature for FRED. */ > + wrmsrns(MSR_IA32_FRED_RSP0, (unsigned long)task_stack_page(task) + THREAD_SIZE); If this non-serializing write happens now and, AFAICT, the CR3 write during the task switch has already happened in switch_mm* earlier, what is the serialization point that's going to make sure that write is committed before the new task starts executing? Thx.
On November 13, 2023 4:37:42 AM EST, Borislav Petkov <bp@alien8.de> wrote: >On Mon, Oct 02, 2023 at 11:24:40PM -0700, Xin Li wrote: >> From: "H. Peter Anvin (Intel)" <hpa@zytor.com> >> >> MSR_IA32_FRED_RSP0 is used during ring 3 event delivery, and needs to >> be updated to point to the top of next task stack during task switch. >> >> 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/include/asm/switch_to.h | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h >> index f42dbf17f52b..c3bd0c0758c9 100644 >> --- a/arch/x86/include/asm/switch_to.h >> +++ b/arch/x86/include/asm/switch_to.h >> @@ -70,9 +70,13 @@ static inline void update_task_stack(struct task_struct *task) >> #ifdef CONFIG_X86_32 >> this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0); >> #else >> - /* Xen PV enters the kernel on the thread stack. */ >> - if (cpu_feature_enabled(X86_FEATURE_XENPV)) >> + if (cpu_feature_enabled(X86_FEATURE_FRED)) { >> + /* WRMSRNS is a baseline feature for FRED. */ >> + wrmsrns(MSR_IA32_FRED_RSP0, (unsigned long)task_stack_page(task) + THREAD_SIZE); > >If this non-serializing write happens now and, AFAICT, the CR3 write >during the task switch has already happened in switch_mm* earlier, what >is the serialization point that's going to make sure that write is >committed before the new task starts executing? > >Thx. > A resource cannot be consumed after the value has been written; this is the only necessary level of serialization, equivalent to, say, RAX. A serializing instruction stops the entire pipeline until everything has retired and any stores have become globally visible.
On Mon, Nov 13, 2023 at 12:36:04PM -0500, H. Peter Anvin wrote: > A resource cannot be consumed after the value has been written; this > is the only necessary level of serialization, equivalent to, say, RAX. Lemme see if I understand this correctly using this context as an example: after this MSR_IA32_FRED_RSP0 write, any FRED events determined to be delivered to level 0 will use this new task stack ptr? And since the new task is not running yet and the old one isn't running either, we're fine here. So the "serialization point" I was talking about above is bollocks. Close? :) > A serializing instruction stops the entire pipeline until everything > has retired and any stores have become globally visible. Right, we don't need that here. Thx.
On November 13, 2023 1:29:47 PM EST, Borislav Petkov <bp@alien8.de> wrote: >On Mon, Nov 13, 2023 at 12:36:04PM -0500, H. Peter Anvin wrote: >> A resource cannot be consumed after the value has been written; this >> is the only necessary level of serialization, equivalent to, say, RAX. > >Lemme see if I understand this correctly using this context as an >example: after this MSR_IA32_FRED_RSP0 write, any FRED events determined >to be delivered to level 0 will use this new task stack ptr? > >And since the new task is not running yet and the old one isn't running >either, we're fine here. So the "serialization point" I was talking >about above is bollocks. > >Close? :) > >> A serializing instruction stops the entire pipeline until everything >> has retired and any stores have become globally visible. > >Right, we don't need that here. > >Thx. > Yep!
diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h index f42dbf17f52b..c3bd0c0758c9 100644 --- a/arch/x86/include/asm/switch_to.h +++ b/arch/x86/include/asm/switch_to.h @@ -70,9 +70,13 @@ static inline void update_task_stack(struct task_struct *task) #ifdef CONFIG_X86_32 this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0); #else - /* Xen PV enters the kernel on the thread stack. */ - if (cpu_feature_enabled(X86_FEATURE_XENPV)) + if (cpu_feature_enabled(X86_FEATURE_FRED)) { + /* WRMSRNS is a baseline feature for FRED. */ + wrmsrns(MSR_IA32_FRED_RSP0, (unsigned long)task_stack_page(task) + THREAD_SIZE); + } else if (cpu_feature_enabled(X86_FEATURE_XENPV)) { + /* Xen PV enters the kernel on the thread stack. */ load_sp0(task_top_of_stack(task)); + } #endif }