On 11/27/2017 02:45 AM, Ingo Molnar wrote:
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1645,11 +1645,13 @@ void cpu_init(void)
> setup_cpu_entry_area(cpu);
>
> /*
> - * Initialize the TSS. Don't bother initializing sp0, as the initial
> - * task never enters user mode.
> + * Initialize the TSS. sp0 points to the entry trampoline stack
> + * regardless of what task is running.
> */
> set_tss_desc(cpu, &get_cpu_entry_area(cpu)->tss.x86_tss);
> load_TR_desc();
> + load_sp0((unsigned long)&get_cpu_entry_area(cpu)->tss +
> + offsetofend(struct tss_struct, SYSENTER_stack));
>
> load_mm_ldt(&init_mm);
Does this also mean that our stack dumps will say "<SYSENTER>" in oopses?
> [ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0
> [ 30.819712] Call Trace:
> [ 30.822442] <SYSENTER>
> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c
...
> [ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60
> [ 31.008533] </SYSENTER>
Should we change that string to something more descriptive?
> On Dec 1, 2017, at 9:06 AM, Dave Hansen <[email protected]> wrote:
>
>> On 11/27/2017 02:45 AM, Ingo Molnar wrote:
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -1645,11 +1645,13 @@ void cpu_init(void)
>> setup_cpu_entry_area(cpu);
>>
>> /*
>> - * Initialize the TSS. Don't bother initializing sp0, as the initial
>> - * task never enters user mode.
>> + * Initialize the TSS. sp0 points to the entry trampoline stack
>> + * regardless of what task is running.
>> */
>> set_tss_desc(cpu, &get_cpu_entry_area(cpu)->tss.x86_tss);
>> load_TR_desc();
>> + load_sp0((unsigned long)&get_cpu_entry_area(cpu)->tss +
>> + offsetofend(struct tss_struct, SYSENTER_stack));
>>
>> load_mm_ldt(&init_mm);
>
> Does this also mean that our stack dumps will say "<SYSENTER>" in oopses?
Only if we oops while we're on the trampoline stack. If the oops is
due to a bug that isn't in the entry code, then we won't see it.
>
>> [ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0
>> [ 30.819712] Call Trace:
>> [ 30.822442] <SYSENTER>
>> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c
> ...
>> [ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60
>> [ 31.008533] </SYSENTER>
>
> Should we change that string to something more descriptive?
I suppose we could rename it to "ENTRY_TRAMPOLINE" or something like that.
>>> [ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0
>>> [ 30.819712] Call Trace:
>>> [ 30.822442] <SYSENTER>
>>> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c
>> ...
>>> [ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60
>>> [ 31.008533] </SYSENTER>
>>
>> Should we change that string to something more descriptive?
>
> I suppose we could rename it to "ENTRY_TRAMPOLINE" or something like that.
The attached patch does just that. Any objections?
On Dec 1, 2017, at 1:21 PM, Dave Hansen <[email protected]> wrote:
>>>> [ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0
>>>> [ 30.819712] Call Trace:
>>>> [ 30.822442] <SYSENTER>
>>>> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c
>>> ...
>>>> [ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60
>>>> [ 31.008533] </SYSENTER>
>>>
>>> Should we change that string to something more descriptive?
>>
>> I suppose we could rename it to "ENTRY_TRAMPOLINE" or something like that.
>
> The attached patch does just that. Any objections?
> <SYSENTER-rename.patch>
I think that, if we do this, we should rename it in the code, too. Calling it one thing in the oops and something else in the code is just going to add confusion.
> From: Dave Hansen <[email protected]>
>
> The "SYSENTER" stack is used for a lot more than SYSENTER now.
> Give it a better string to display in stack dumps.
>
> We should probably cleanse the 64-bit code of the remaining
> "SYSENTER" nomenclature too at some point.
>
> Signed-off-by: Dave Hansen <[email protected]>
> ---
>
> b/arch//x86/kernel/dumpstack_64.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff -puN arch//x86/kernel/dumpstack_64.c~SYSENTER-rename arch//x86/kernel/dumpstack_64.c
> --- a/arch//x86/kernel/dumpstack_64.c~SYSENTER-rename 2017-12-01 12:43:16.768707737 -0800
> +++ b/arch//x86/kernel/dumpstack_64.c 2017-12-01 13:19:21.741702337 -0800
> @@ -37,8 +37,14 @@ const char *stack_type_name(enum stack_t
> if (type == STACK_TYPE_IRQ)
> return "IRQ";
>
> - if (type == STACK_TYPE_SYSENTER)
> - return "SYSENTER";
> + if (type == STACK_TYPE_SYSENTER) {
> + /*
> + * On 64-bit, we have a generic entry stack that we
> + * use for all the kernel try points, including
> + * SYSENTER.
ITYM "kernel entry points".
- Kevin