From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753311AbdKWT4I (ORCPT ); Thu, 23 Nov 2017 14:56:08 -0500 Received: from mx2.suse.de ([195.135.220.15]:54148 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753229AbdKWT4G (ORCPT ); Thu, 23 Nov 2017 14:56:06 -0500 Date: Thu, 23 Nov 2017 20:55:55 +0100 From: Borislav Petkov To: Andy Lutomirski Cc: X86 ML , "linux-kernel@vger.kernel.org" , Brian Gerst , Dave Hansen , Linus Torvalds , Josh Poimboeuf Subject: Re: [PATCH v2 10/18] x86/asm: Remap the TSS into the cpu entry area Message-ID: <20171123195555.ct72tiphayavkffq@pd.tnic> References: <07895bbccbff01d5c6ba3b1a5bf48fab34b92eae.1511325444.git.luto@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <07895bbccbff01d5c6ba3b1a5bf48fab34b92eae.1511325444.git.luto@kernel.org> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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. > 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. And the comments are not really needed - the variable names already say what they are. ... > @@ -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); > > 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) --