From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander van Heukelum Subject: Re: [PATCH 2/3] x86: tss: Eliminate fragile calculation of TSS segment limit Date: Thu, 31 Oct 2013 21:02:14 +0100 Message-ID: <1383249734.5399.41325681.3DEA6791@webmail.messagingengine.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Josh Triplett , linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, x86@kernel.org, xen-devel@lists.xenproject.org Cc: Jeremy Fitzhardinge , Raghavendra K T , Len Brown , Frederic Weisbecker , "H. Peter Anvin" , Paul Gortmaker , David Herrmann , Masami Hiramatsu , Seiji Aguchi , Jiri Slaby , Alok Kataria , Jesper Nilsson , Andi Kleen , Daniel Lezcano , Ingo Molnar , Steven Rostedt , Borislav Petkov , Fenghua Yu , Kees Cook , Konrad Rzeszutek Wilk , Ross Lagerwall , Chris Metcalf , Chris Wright List-Id: virtualization@lists.linuxfoundation.org Hi Josh, On Tue, Oct 22, 2013, at 3:34, Josh Triplett wrote: > __set_tss_desc has a complex calculation of the TSS segment limit, > duplicating the quirky details of the I/O bitmap array length, and > requiring a complex comment to explain. Replace that calculation with a > simpler one based on the offsetof the "stack" field that follows the > array. > > That then removes the last use of IO_BITMAP_OFFSET, so delete it. > > Signed-off-by: Josh Triplett > --- > arch/x86/include/asm/desc.h | 11 +---------- > arch/x86/include/asm/processor.h | 3 ++- > 2 files changed, 3 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h > index b90e5df..17ac92f 100644 > --- a/arch/x86/include/asm/desc.h > +++ b/arch/x86/include/asm/desc.h > @@ -177,16 +177,7 @@ static inline void __set_tss_desc(unsigned cpu, unsigned int entry, void *addr) > struct desc_struct *d = get_cpu_gdt_table(cpu); > tss_desc tss; > > - /* > - * sizeof(unsigned long) coming from an extra "long" at the end > - * of the iobitmap. See tss_struct definition in processor.h > - * > - * -1? seg base+limit should be pointing to the address of the > - * last valid byte > - */ > - set_tssldt_descriptor(&tss, (unsigned long)addr, DESC_TSS, > - IO_BITMAP_OFFSET + IO_BITMAP_BYTES + > - sizeof(unsigned long) - 1); > + set_tssldt_descriptor(&tss, (unsigned long)addr, DESC_TSS, TSS_LIMIT); > write_gdt_entry(d, entry, &tss, DESC_TSS); > } > > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index 987c75e..03d3003 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -259,9 +259,10 @@ struct x86_hw_tss { > #define IO_BITMAP_BITS 65536 > #define IO_BITMAP_BYTES (IO_BITMAP_BITS/8) > #define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long)) > -#define IO_BITMAP_OFFSET offsetof(struct tss_struct, io_bitmap) > #define INVALID_IO_BITMAP_OFFSET 0x8000 > > +#define TSS_LIMIT (offsetof(struct tss_struct, stack) - 1) > + I wonder if the 'stack' in tss_struct has any meaning anymore. Can it be removed? If so, the offsetof can be changed to sizeof(struct tss_struct), which looks more natural to me. Even so, I think this is already an improvement. Acked-by: Alexander van Heukelum > struct tss_struct { > /* > * The hardware state: > -- > 1.8.4.rc3 >