diff mbox series

[RFC,23/32] x86/fred: update MSR_IA32_FRED_RSP0 during task switch

Message ID 20221220063658.19271-24-xin3.li@intel.com
State New
Headers
Series x86: enable FRED for x86-64 |

Commit Message

Li, Xin3 Dec. 20, 2022, 6:36 a.m. UTC 
  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>
Signed-off-by: Xin Li <xin3.li@intel.com>
---
 arch/x86/include/asm/switch_to.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Peter Zijlstra Dec. 20, 2022, 9:48 a.m. UTC | #1 
On Mon, Dec 19, 2022 at 10:36:49PM -0800, 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>
> 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 c08eb0fdd11f..c28170d4fbba 100644
> --- a/arch/x86/include/asm/switch_to.h
> +++ b/arch/x86/include/asm/switch_to.h
> @@ -71,9 +71,13 @@ static inline void update_task_stack(struct task_struct *task)
>  	else
>  		this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0);
>  #else
> -	/* Xen PV enters the kernel on the thread stack. */
> -	if (static_cpu_has(X86_FEATURE_XENPV))
> +	if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> +		wrmsrl(MSR_IA32_FRED_RSP0,
> +		       task_top_of_stack(task) + TOP_OF_KERNEL_STACK_PADDING);

Urgh, I'm assuming this is a *fast* MSR ?

> +	} else if (static_cpu_has(X86_FEATURE_XENPV)) {
> +		/* Xen PV enters the kernel on the thread stack. */
>  		load_sp0(task_top_of_stack(task));
> +	}
>  #endif
Li, Xin3 Dec. 20, 2022, 6:47 p.m. UTC | #2 
> > --- a/arch/x86/include/asm/switch_to.h
> > +++ b/arch/x86/include/asm/switch_to.h
> > @@ -71,9 +71,13 @@ static inline void update_task_stack(struct task_struct
> *task)
> >  	else
> >  		this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0);  #else
> > -	/* Xen PV enters the kernel on the thread stack. */
> > -	if (static_cpu_has(X86_FEATURE_XENPV))
> > +	if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> > +		wrmsrl(MSR_IA32_FRED_RSP0,
> > +		       task_top_of_stack(task) + TOP_OF_KERNEL_STACK_PADDING);
> 
> Urgh, I'm assuming this is a *fast* MSR ?

You're right, however that is another feature under development AFAIK.

Xin
H. Peter Anvin Dec. 23, 2022, 7:42 p.m. UTC | #3 
On December 20, 2022 1:48:44 AM PST, Peter Zijlstra <peterz@infradead.org> wrote:
>On Mon, Dec 19, 2022 at 10:36:49PM -0800, 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>
>> 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 c08eb0fdd11f..c28170d4fbba 100644
>> --- a/arch/x86/include/asm/switch_to.h
>> +++ b/arch/x86/include/asm/switch_to.h
>> @@ -71,9 +71,13 @@ static inline void update_task_stack(struct task_struct *task)
>>  	else
>>  		this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0);
>>  #else
>> -	/* Xen PV enters the kernel on the thread stack. */
>> -	if (static_cpu_has(X86_FEATURE_XENPV))
>> +	if (cpu_feature_enabled(X86_FEATURE_FRED)) {
>> +		wrmsrl(MSR_IA32_FRED_RSP0,
>> +		       task_top_of_stack(task) + TOP_OF_KERNEL_STACK_PADDING);
>
>Urgh, I'm assuming this is a *fast* MSR ?
>
>> +	} else if (static_cpu_has(X86_FEATURE_XENPV)) {
>> +		/* Xen PV enters the kernel on the thread stack. */
>>  		load_sp0(task_top_of_stack(task));
>> +	}
>>  #endif
>
>

The performance here will be addressed by WRMSRNS/WRMSRLIST. It is not included in the FRED patchset simply because there is a separate, parallel enabling effort going on for those instructions (which are useful in their own right, especially for perf, and may be available before FRED) and we don't want unnecessary collisions.

Those instructions weren't public when I wrote the first version of this patchset, but they are now in the ISE documentation.

Xin, could you add that note to the patch documentation?
Li, Xin3 Dec. 24, 2022, 3:02 a.m. UTC | #4 
> >> diff --git a/arch/x86/include/asm/switch_to.h
> >> b/arch/x86/include/asm/switch_to.h
> >> index c08eb0fdd11f..c28170d4fbba 100644
> >> --- a/arch/x86/include/asm/switch_to.h
> >> +++ b/arch/x86/include/asm/switch_to.h
> >> @@ -71,9 +71,13 @@ static inline void update_task_stack(struct task_struct
> *task)
> >>  	else
> >>  		this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0);  #else
> >> -	/* Xen PV enters the kernel on the thread stack. */
> >> -	if (static_cpu_has(X86_FEATURE_XENPV))
> >> +	if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> >> +		wrmsrl(MSR_IA32_FRED_RSP0,
> >> +		       task_top_of_stack(task) +
> TOP_OF_KERNEL_STACK_PADDING);
> >
> >Urgh, I'm assuming this is a *fast* MSR ?
> >
> >> +	} else if (static_cpu_has(X86_FEATURE_XENPV)) {
> >> +		/* Xen PV enters the kernel on the thread stack. */
> >>  		load_sp0(task_top_of_stack(task));
> >> +	}
> >>  #endif
> >
> >
> 
> The performance here will be addressed by WRMSRNS/WRMSRLIST. It is not
> included in the FRED patchset simply because there is a separate, parallel
> enabling effort going on for those instructions (which are useful in their own
> right, especially for perf, and may be available before FRED) and we don't want
> unnecessary collisions.
> 
> Those instructions weren't public when I wrote the first version of this patchset,
> but they are now in the ISE documentation.
> 
> Xin, could you add that note to the patch documentation?

Will do!
diff mbox series

Patch

diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h
index c08eb0fdd11f..c28170d4fbba 100644
--- a/arch/x86/include/asm/switch_to.h
+++ b/arch/x86/include/asm/switch_to.h
@@ -71,9 +71,13 @@  static inline void update_task_stack(struct task_struct *task)
 	else
 		this_cpu_write(cpu_tss_rw.x86_tss.sp1, task->thread.sp0);
 #else
-	/* Xen PV enters the kernel on the thread stack. */
-	if (static_cpu_has(X86_FEATURE_XENPV))
+	if (cpu_feature_enabled(X86_FEATURE_FRED)) {
+		wrmsrl(MSR_IA32_FRED_RSP0,
+		       task_top_of_stack(task) + TOP_OF_KERNEL_STACK_PADDING);
+	} else if (static_cpu_has(X86_FEATURE_XENPV)) {
+		/* Xen PV enters the kernel on the thread stack. */
 		load_sp0(task_top_of_stack(task));
+	}
 #endif
 }