From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756451AbbEVGWc (ORCPT ); Fri, 22 May 2015 02:22:32 -0400 Received: from mail-wg0-f46.google.com ([74.125.82.46]:34783 "EHLO mail-wg0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751430AbbEVGW3 (ORCPT ); Fri, 22 May 2015 02:22:29 -0400 Date: Fri, 22 May 2015 08:22:24 +0200 From: Ingo Molnar To: Andy Lutomirski Cc: x86@kernel.org, "H. Peter Anvin" , "H.J. Lu" , Borislav Petkov , Jan Beulich , Binutils , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v2] x86: Stop relying on magic jmp behavior for early_idt_handlers Message-ID: <20150522062224.GA4736@gmail.com> References: <9b741597a52258e829bae247216da656d452395a.1432257964.git.luto@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9b741597a52258e829bae247216da656d452395a.1432257964.git.luto@kernel.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Andy Lutomirski wrote: > --- a/arch/x86/include/asm/segment.h > +++ b/arch/x86/include/asm/segment.h > @@ -231,9 +231,17 @@ > #define TLS_SIZE (GDT_ENTRY_TLS_ENTRIES* 8) > > #ifdef __KERNEL__ > + > +/* > + * early_idt_handlers is an array of entry points. For simplicity, it's > + * a real array. We allocate nine bytes for each entry: two one-byte > + * push instructions and a five-byte jump in the worst case. > + */ > +#define EARLY_IDT_HANDLER_STRIDE 9 So how come that two plus five equals nine? ;-) It's two two-bype pushes (sometimes a 2-byte NOP), plus a 5-byte jump, in the worst case. I'd also mention that it's an array of 32 small trampolines that set up parameters and jump to a common entry point. > +/* Build the early_idt_handlers array */ > ENTRY(early_idt_handlers) Please rename the function accordingly: early_idt_handlers_array, to make clear this is never jumped to directly, only through the IDT. > # 36(%esp) %eflags > # 32(%esp) %cs > @@ -531,19 +532,18 @@ ENTRY(early_idt_handlers) > # 24(%rsp) error code > i = 0 > .rept NUM_EXCEPTION_VECTORS > - .if (EXCEPTION_ERRCODE_MASK >> i) & 1 > - ASM_NOP2 > - .else > + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc > + .ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1 > pushl $0 # Dummy error code, to make stack frame uniform > .endif > pushl $i # 20(%esp) Vector number > jmp early_idt_handler > i = i + 1 > .endr > + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc > ENDPROC(early_idt_handlers) > > - /* This is global to keep gas from relaxing the jumps */ > -ENTRY(early_idt_handler) > +early_idt_handler: Please rename this as well to make it clearer what's happening, something like: early_idt_handler_common: would work for me. Ditto for the 64-bit side. Thanks, Ingo