From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josh Triplett Subject: Re: [PATCH 2/3] x86: tss: Eliminate fragile calculation of TSS segment limit Date: Thu, 31 Oct 2013 04:12:43 -0700 Message-ID: <20131031111243.GA25280@leaf> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline 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: Kees Cook Cc: Alexander van Heukelum , Jeremy Fitzhardinge , "x86@kernel.org" , Len Brown , Frederic Weisbecker , "H. Peter Anvin" , "virtualization@lists.linux-foundation.org" , Paul Gortmaker , Raghavendra K T , David Herrmann , Masami Hiramatsu , Seiji Aguchi , Jiri Slaby , Alok Kataria , Jesper Nilsson , Andi Kleen , Daniel Lezcano , Ingo Molnar , Steven Rostedt , xen-devel@lists.xenproject.org, Borislav Petkov , Fenghua Yu List-Id: virtualization@lists.linuxfoundation.org On Wed, Oct 30, 2013 at 03:22:33PM -0700, Kees Cook wrote: > On Mon, Oct 21, 2013 at 7:34 PM, 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 > > I think it might be better to keep at least a minimal comment near the > TSS_LIMIT declaration, just to explain the "-1" part, which is not > entirely obvious from just reading the code. Fair enough; I've added an appropriate comment next to TSS_LIMIT, and I'll include that in PATCHv2, which I'll send out as soon as I see any feedback on patch 3/3. - Josh Triplett