From patchwork Fri Mar 9 14:30:41 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Whitehorn X-Patchwork-Id: 145722 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 1B9D7B6EF3 for ; Sat, 10 Mar 2012 01:31:08 +1100 (EST) Received: from localhost ([::1]:34341 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S60qY-0001EI-0R for incoming@patchwork.ozlabs.org; Fri, 09 Mar 2012 09:31:06 -0500 Received: from eggs.gnu.org ([208.118.235.92]:47870) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S60qN-00014y-VG for qemu-devel@nongnu.org; Fri, 09 Mar 2012 09:30:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S60qD-0002uK-Od for qemu-devel@nongnu.org; Fri, 09 Mar 2012 09:30:55 -0500 Received: from argol.doit.wisc.edu ([144.92.197.212]:45187) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S60qD-0002tl-JD; Fri, 09 Mar 2012 09:30:45 -0500 MIME-version: 1.0 Content-type: multipart/mixed; boundary="Boundary_(ID_llPSGvUGxTHSO8A3M1t8xQ)" Received: from avs-daemon.smtpauth3.wiscmail.wisc.edu by smtpauth3.wiscmail.wisc.edu (Sun Java(tm) System Messaging Server 7u2-7.05 32bit (built Jul 30 2009)) id <0M0M00902GB7Q800@smtpauth3.wiscmail.wisc.edu>; Fri, 09 Mar 2012 08:30:43 -0600 (CST) Received: from comporellon.tachypleus.net ([unknown] [76.210.73.151]) by smtpauth3.wiscmail.wisc.edu (Sun Java(tm) System Messaging Server 7u2-7.05 32bit (built Jul 30 2009)) with ESMTPSA id <0M0M000J4GB5BR20@smtpauth3.wiscmail.wisc.edu>; Fri, 09 Mar 2012 08:30:42 -0600 (CST) Date: Fri, 09 Mar 2012 08:30:41 -0600 From: Nathan Whitehorn In-reply-to: To: Alexander Graf Message-id: <4F5A1411.2040300@freebsd.org> References: <4F524946.1050001@freebsd.org> <20120308012543.GB10735@truffala.fritz.box> <20120309034255.GQ10735@truffala.fritz.box> User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:10.0) Gecko/20120212 Thunderbird/10.0 X-detected-operating-system: by eggs.gnu.org: Solaris 10 (beta) X-Received-From: 144.92.197.212 Cc: qemu-ppc@nongnu.org, QEMU Developers , David Gibson Subject: Re: [Qemu-devel] [PATCH] PPC: Fix large page support in TCG X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 03/09/12 07:13, Alexander Graf wrote: > On 09.03.2012, at 04:42, David Gibson wrote: > >> On Thu, Mar 08, 2012 at 09:24:53AM -0600, Nathan Whitehorn wrote: >>> On Mar 7, 2012, at 7:25 PM, David Gibson wrote: >>> >>>> On Sat, Mar 03, 2012 at 10:39:34AM -0600, Nathan Whitehorn wrote: >>>>> Fix large page support in TCG. The old code would overwrite the >>>>> large page table entry with the fake 4 KB >>>>> one generated here whenever the ref/change bits were updated, >>>>> causing it to point to the wrong area of memory. Instead of creating >>>>> a fake PTE, just update the real address at the end. >>>>> >>>>> Signed-off-by: Nathan Whitehorn >>>> Hrm. This looks like a cleaner way of handling things, but I don't >>>> really follow what exactly was going wrong in the old way. Can you >>>> spell out in more detail where the modified pte1 value caused >>>> problems? >>> The problem was that pte1 would get extra bits added into it in >>> _find_pte() to produce a new, fake 4KB page table entry. When the >>> ref/change bits were updated, pte1 would be written back to the page >>> table -- *including* the bits added to make a fake 4K page. At the >>> next access, since this function does not clear the low bits of >>> large pages (which is probably itself a bug) when it interprets >>> them, the generated address would be the large page base, ored with >>> the large page remainder for this access, ored with the large page >>> remainder from the *previous* access, etc. and you would get a >>> progressively more bogus address in the end. >> Ah, yes, I see it now. Good catch. >> >> Acked-by: David Gibson > Hrm - the patch doesn't apply for me. Could you please resend as something that's applyable? :) > Also, please make sure to always CC qemu-ppc on ppc patches, otherwise there's a good chance they slip off my radar. > > > Alex > Weird. I've provided it as an attachment, which should hopefully work this time. -Nathan Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Signed-off-by: Nathan Whitehorn --- target-ppc/helper.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 928fbcf..0f5ad2e 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -597,12 +597,6 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h, pte1 = ldq_phys(env->htab_base + pteg_off + (i * 16) + 8); } - /* We have a TLB that saves 4K pages, so let's - * split a huge page to 4k chunks */ - if (target_page_bits != TARGET_PAGE_BITS) - pte1 |= (ctx->eaddr & (( 1 << target_page_bits ) - 1)) - & TARGET_PAGE_MASK; - r = pte64_check(ctx, pte0, pte1, h, rw, type); LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " " TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n", @@ -678,6 +672,11 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h, } } + /* We have a TLB that saves 4K pages, so let's + * split a huge page to 4k chunks */ + if (target_page_bits != TARGET_PAGE_BITS) + ctx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1)) + & TARGET_PAGE_MASK; return ret; }