From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RpRdZ-0004IG-BS for qemu-devel@nongnu.org; Mon, 23 Jan 2012 16:41:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RpRdW-0004jL-V3 for qemu-devel@nongnu.org; Mon, 23 Jan 2012 16:41:12 -0500 Message-ID: <4F1DD3ED.50101@freescale.com> Date: Mon, 23 Jan 2012 15:41:01 -0600 From: Scott Wood MIME-Version: 1.0 References: <1327119330-29304-1-git-send-email-agraf@suse.de> <1327119330-29304-8-git-send-email-agraf@suse.de> <4F1D99A1.7080003@freescale.com> <4F1D99FF.4010905@suse.de> <4F1DA4A5.6060202@freescale.com> <4F1DA9DB.3020707@suse.de> <4F1DABBD.6040108@freescale.com> <4F1DBEC5.5070406@freescale.com> <353F52B3-BC7E-471B-BB8A-9DA6C06C2A59@suse.de> In-Reply-To: <353F52B3-BC7E-471B-BB8A-9DA6C06C2A59@suse.de> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 7/8] PPC: booke206: Check for min/max TLB entry size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: "" , qemu-devel Developers On 01/23/2012 03:29 PM, Alexander Graf wrote: > > > On 23.01.2012, at 21:10, Scott Wood wrote: > >> If TLB0 has TLBnCFG[AVAIL] set, then with this patch you'll be raising >> an exception rather than setting the size to the minimum. >> >> If TLB0 does not have TLBnCFG[AVAIL] set, you'll be letting the user set >> whatever size they want. >> >> In either case, you seem to be letting the user write whatever the want >> to the TLB array, and only afterward check whether to send an exception. > > Yes, for !AVAIL we simply override the page size on qemu tlb miss iirc. Ah. That seems like a hotter path than tlbwe, and you could still insert an invalid entry into tlb1 (you'd get an exception, but the entry would be there). > Is that wrong? Does tlbwe;tlbre result in different tsize values? e500mc manual (table 6-6, "MMU Assist Register Field Updates") says tlbre returns a tsize of 1 for tlb0 -- it doesn't store tsize. The KVM MMU API also requires that tsize be stored as a valid value, to simplify the code that operates on the TLB. The TLB dump code depends on this (could be fixed of course, but simpler to fix it once in tlbwe). >>> True. Maybe we should just always reserve a surplus TLB entry and have the current code work, basically making it be a nop? >>> >>> Or we could add checks everywhere... >> >> I'd have booke206_get_tlbm() check and return NULL, with callers >> checking for that. Optimization can come later, if/when it's shown to >> be a bottleneck. > > It's more about not missing any cases :). But yeah, it's probably best to just change the semantics. At least a NULL deference will be more noticeable than an array overrun... -Scott