Messages in this thread | | | From | Andy Lutomirski <> | Date | Thu, 23 Nov 2017 12:15:14 -0800 | Subject | Re: [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) > --
| |