Date: Thu, 23 Jul 2015 12:06:50 -0400
From: Steven Rostedt <rostedt@goodmis.org>
To: Andy Lutomirski <luto@kernel.org>
Cc: X86 ML <x86@kernel.org>, linux-kernel@vger.kernel.org,
        Willy Tarreau <w@1wt.eu>, Borislav Petkov <bp@alien8.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 3/5] x86/entry: Merge 32-bit and 64-bit sysenter setup
 code
Message-ID: <20150723120650.2257cd8c@gandalf.local.home>
In-Reply-To: <0ed9e2434ed6b0832b0f1b28407ad962f66893a7.1437665045.git.luto@kernel.org>
References: <cover.1437665045.git.luto@kernel.org>
	<0ed9e2434ed6b0832b0f1b28407ad962f66893a7.1437665045.git.luto@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2495
Lines: 70

On Thu, 23 Jul 2015 08:31:41 -0700
Andy Lutomirski <luto@kernel.org> wrote:


> -	if (!boot_cpu_has(X86_FEATURE_SEP))
> +	/*
> +	 * On 64-bit CPUs, enable SEP unconditionally.  On Intel CPUs,
> +	 * it works and we use it.  On AMD CPUs, the MSRs exist but EIP
> +	 * is truncated to 32 bits.  This doesn't matter because AMD
> +	 * CPUs disallow SYSENTER in long mode.  If AMD ever decides to
> +	 * support SYSENTER, then they'll have to fix the truncation
> +	 * issue, and this code will work as-is.
> +	 */
> +
> +	if (IS_ENABLED(CONFIG_X86_32) && !boot_cpu_has(X86_FEATURE_SEP))
>  		goto out;
>  
> +#ifdef CONFIG_X86_32
>  	/*
>  	 * We cache MSR_IA32_SYSENTER_CS's value in the TSS's ss1 field --
>  	 * see the big comment in struct x86_hw_tss's definition.
>  	 */
>  	tss->x86_tss.ss1 = __KERNEL_CS;
> +#endif
>  
>  	wrmsrl_safe(MSR_IA32_SYSENTER_CS, __KERNEL_CS);
>  	wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
>  		    (unsigned long)tss +
>  		    offsetofend(struct tss_struct, SYSENTER_stack));
> +#ifdef CONFIG_X86_32
>  	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32);
> +#else
> +	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_compat);
> +#endif

As an additional clean up, what impact would we have to just rename
entry_SYSENTER_compat to entry_SYSENTER_32 on x86_64? It would remove
the need for the above #ifdef logic.

-- Steve


>  
>  out:
>  	put_cpu();
> @@ -1187,17 +1201,7 @@ void syscall_init(void)
>  
>  #ifdef CONFIG_IA32_EMULATION
>  	wrmsrl(MSR_CSTAR, entry_SYSCALL_compat);
> -	/*
> -	 * This only works on Intel CPUs.
> -	 * On AMD CPUs these MSRs are 32-bit, CPU truncates MSR_IA32_SYSENTER_EIP.
> -	 * This does not cause SYSENTER to jump to the wrong location, because
> -	 * AMD doesn't allow SYSENTER in long mode (either 32- or 64-bit).
> -	 */
> -	wrmsrl_safe(MSR_IA32_SYSENTER_CS, __KERNEL_CS);
> -	wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
> -		    (unsigned long)&per_cpu(cpu_tss, smp_processor_id()) +
> -		    offsetofend(struct tss_struct, SYSENTER_stack));
> -	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_compat);
> +	enable_sep_cpu();
>  #else
>  	wrmsrl(MSR_CSTAR, ignore_sysret);
>  	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/