From mboxrd@z Thu Jan 1 00:00:00 1970 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 MIME-version: 1.0 Content-type: multipart/mixed; boundary="Boundary_(ID_llPSGvUGxTHSO8A3M1t8xQ)" Date: Fri, 09 Mar 2012 08:30:41 -0600 From: Nathan Whitehorn In-reply-to: Message-id: <4F5A1411.2040300@freebsd.org> References: <4F524946.1050001@freebsd.org> <20120308012543.GB10735@truffala.fritz.box> <20120309034255.GQ10735@truffala.fritz.box> Subject: Re: [Qemu-devel] [PATCH] PPC: Fix large page support in TCG List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: qemu-ppc@nongnu.org, QEMU Developers , David Gibson This is a multi-part message in MIME format. --Boundary_(ID_llPSGvUGxTHSO8A3M1t8xQ) Content-type: text/plain; CHARSET=US-ASCII; format=flowed Content-transfer-encoding: 7BIT 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 --Boundary_(ID_llPSGvUGxTHSO8A3M1t8xQ) Content-type: text/plain; name=0005-Fix-large-page-support-in-TCG.-The-old-code-would-ov.patch Content-transfer-encoding: 7BIT Content-disposition: attachment; filename*0=0005-Fix-large-page-support-in-TCG.-The-; filename*1=old-code-would-ov.patch 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; } -- 1.7.9 --Boundary_(ID_llPSGvUGxTHSO8A3M1t8xQ)--