lkml.org 
[lkml]   [2017]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 10/18] x86/asm: Remap the TSS into the cpu entry area
On Thu, Nov 23, 2017 at 11:55 AM, Borislav Petkov <bp@suse.de> wrote:
> On Tue, Nov 21, 2017 at 08:44:05PM -0800, Andy Lutomirski wrote:
>> This has a secondary purpose: it puts the entry stack into a region
>> with a well-controlled layout. A subsequent patch will take
>> advantage of this to streamline the SYSCALL entry code to be able to
>> find it more easily.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>> arch/x86/entry/entry_32.S | 6 ++++--
>> arch/x86/include/asm/fixmap.h | 7 +++++++
>> arch/x86/kernel/asm-offsets.c | 3 +++
>> arch/x86/kernel/cpu/common.c | 40 ++++++++++++++++++++++++++++++++++------
>> arch/x86/kernel/dumpstack.c | 3 ++-
>> arch/x86/power/cpu.c | 11 ++++++-----
>> 6 files changed, 56 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
>> index 4838037f97f6..0ab316c46806 100644
>> --- a/arch/x86/entry/entry_32.S
>> +++ b/arch/x86/entry/entry_32.S
>> @@ -941,7 +941,8 @@ ENTRY(debug)
>> movl %esp, %eax # pt_regs pointer
>>
>> /* Are we currently on the SYSENTER stack? */
>> - PER_CPU(cpu_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx)
>> + movl PER_CPU_VAR(cpu_entry_area), %ecx
>> + addl $CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx
>> subl %eax, %ecx /* ecx = (end of SYSENTER_stack) - esp */
>
> Fixup that comment while you're at it to be either ontop or be of the
> #-type. The cool thing about the # comments is that gas puts them in the
> .s output too, so you can find them again.
>
>> cmpl $SIZEOF_SYSENTER_stack, %ecx
>> jb .Ldebug_from_sysenter_stack
>> @@ -984,7 +985,8 @@ ENTRY(nmi)
>> movl %esp, %eax # pt_regs pointer
>>
>> /* Are we currently on the SYSENTER stack? */
>> - PER_CPU(cpu_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx)
>> + movl PER_CPU_VAR(cpu_entry_area), %ecx
>> + addl $CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx
>> subl %eax, %ecx /* ecx = (end of SYSENTER_stack) - esp */
>
> Ditto.

Nifty. I didn't know about that.

>
>> cmpl $SIZEOF_SYSENTER_stack, %ecx
>> jb .Lnmi_from_sysenter_stack
>
> ...
>
>> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
>> index b275863128eb..55858b277cf6 100644
>> --- a/arch/x86/kernel/asm-offsets.c
>> +++ b/arch/x86/kernel/asm-offsets.c
>> @@ -98,4 +98,7 @@ void common(void) {
>> OFFSET(CPU_TSS_SYSENTER_stack, tss_struct, SYSENTER_stack);
>> /* Size of SYSENTER_stack */
>> DEFINE(SIZEOF_SYSENTER_stack, sizeof(((struct tss_struct *)0)->SYSENTER_stack));
>> +
>> + /* Layout info for cpu_entry_area */
>> + OFFSET(CPU_ENTRY_AREA_tss, cpu_entry_area, tss);
>
> The naming convention of those looks really strange. The beginning is in
> all caps and then small. Can we choose a convention pls.

This is actually consistent with the rest of the offsets. The
convention is STRUCT_field.

>
> And the comments are not really needed - the variable names already say
> what they are.

Fair enough.

>
> ...
>
>> @@ -1281,7 +1304,8 @@ void enable_sep_cpu(void)
>> wrmsr(MSR_IA32_SYSENTER_CS, tss->x86_tss.ss1, 0);
>>
>> wrmsr(MSR_IA32_SYSENTER_ESP,
>> - (unsigned long)tss + offsetofend(struct tss_struct, SYSENTER_stack),
>> + (unsigned long)&get_cpu_entry_area(cpu)->tss +
>> + offsetofend(struct tss_struct, SYSENTER_stack),
>> 0);
>
> Use a local var:
>
> unsigned long se_esp;
>
> ...
>
> se_esp = (unsigned long)&get_cpu_entry_area(cpu)->tss + offsetofend(struct tss_struct, SYSENTER_stack);
>
> wrmsr(MSR_IA32_SYSENTER_ESP, se_esp, 0);

Hmm. I may just add a helper in a header for this. It has plenty of users.

>
>>
>> wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32, 0);
>> @@ -1394,6 +1418,8 @@ static DEFINE_PER_CPU_PAGE_ALIGNED(char, exception_stacks
>> /* May not be marked __init: used by software suspend */
>> void syscall_init(void)
>> {
>> + int cpu = smp_processor_id();
>> +
>> wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
>> wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
>>
>> @@ -1407,7 +1433,7 @@ void syscall_init(void)
>> */
>> wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)__KERNEL_CS);
>> wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
>> - (unsigned long)this_cpu_ptr(&cpu_tss) +
>> + (unsigned long)&get_cpu_entry_area(cpu)->tss +
>> offsetofend(struct tss_struct, SYSENTER_stack));
>
> Ditto.
>
>> wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
>> #else
>
> --
> Regards/Gruss,
> Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> --

\
 
 \ /
  Last update: 2017-11-23 21:15    [W:0.086 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site