From patchwork Fri Oct 2 13:06:34 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joakim Tjernlund X-Patchwork-Id: 34854 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from bilbo.ozlabs.org (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 9C64DB7CB6 for ; Fri, 2 Oct 2009 23:07:07 +1000 (EST) Received: by ozlabs.org (Postfix) id BB9FBB7BDE; Fri, 2 Oct 2009 23:07:01 +1000 (EST) Delivered-To: linuxppc-dev@ozlabs.org Received: from gw1.transmode.se (gw1.transmode.se [213.115.205.20]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EBCA2B7BDA for ; Fri, 2 Oct 2009 23:06:59 +1000 (EST) Received: from sesr04.transmode.se (sesr04.transmode.se [192.168.201.15]) by gw1.transmode.se (Postfix) with ESMTP id 551E4650001; Fri, 2 Oct 2009 15:06:54 +0200 (CEST) In-Reply-To: <1254350159.5699.21.camel@pasglop> References: <1253843480.7103.492.camel@pasglop> <1254208057.5771.7.camel@pasglop> <1254212198.5256.0.camel@pasglop> <20090929210331.GA25779@laura.chatsunix.int.mrv.com> <20090930090002.GA2928@compile2.chatsunix.int.mrv.com> <1254350159.5699.21.camel@pasglop> Subject: Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite X-KeepSent: 2FCFEA43:75290607-C1257643:0047C81F; type=4; name=$KeepSent To: Benjamin Herrenschmidt X-Mailer: Lotus Notes Release 8.5 December 05, 2008 Message-ID: From: Joakim Tjernlund Date: Fri, 2 Oct 2009 15:06:34 +0200 X-MIMETrack: Serialize by Router on sesr04/Transmode(Release 8.5 HF407|May 07, 2009) at 2009-10-02 15:06:54 MIME-Version: 1.0 Cc: "linuxppc-dev@ozlabs.org" , Rex Feany X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Benjamin Herrenschmidt wrote on 01/10/2009 00:35:59: > > > > > Had a look at linus tree and there is something I don't understand. > > > Your fix, e0908085fc2391c85b85fb814ae1df377c8e0dcb, fixes a problem > > > that was introduced by 8d30c14cab30d405a05f2aaceda1e9ad57800f36 but > > > 8d30c14cab30d405a05f2aaceda1e9ad57800f36 was included in .31 and .31 > > > works and top of tree does not, how can that be? > > > > > > To me it seems more likely that some mm change introduced between .31 and > > > top of tree is the culprit. > > > My patch addresses the problem described in the comment: > > > /* On 8xx, cache control instructions (particularly > > > * "dcbst" from flush_dcache_icache) fault as write > > > * operation if there is an unpopulated TLB entry > > > * for the address in question. To workaround that, > > > * we invalidate the TLB here, thus avoiding dcbst > > > * misbehaviour. > > > */ > > > Now you are using this old fix to paper over some other bug or so I think. > > > > There is something fishy with the TLB status, looking into the mpc862 manual I > > don't see how it can work reliably. Need to dwell some more. > > Anyhow, I have incorporated some of my findings into a new patch, > > perhaps this will be the golden one? > > Well, I still wonder about what whole unpopulated entry business. > > >From what I can see, the TLB miss code will check _PAGE_PRESENT, and > when not set, it will -still- insert something into the TLB (unlike > all other CPU types that go straight to data access faults from there). > > So we end up populating with those unpopulated entries that will then > cause us to take a DSI (or ISI) instead of a TLB miss the next time > around ... and so we need to remove them once we do that, no ? IE. Once > we have actually put a valid PTE in. > > At least that's my understanding and why I believe we should always have > tlbil_va() in set_pte() and ptep_set_access_flags(), which boils down > in putting it into the 2 "filter" functions in the new code. > > Well.. actually, the ptep_set_access_flags() will already do a > flush_tlb_page_nohash() when the PTE is changed. So I suppose all we > really need here would be in set_pte_filter(), to do the same if we are > writing a PTE that has _PAGE_PRESENT, at least on 8xx. > > But just unconditionally doing a tlbil_va() in both the filter functions > shouldn't harm and also fix the problem, though Rex seems to indicate > that is not the case. > > Now, we -might- have something else broken on 8xx, hard to tell. You may > want to try to bisect, adding back the removed tlbil_va() as you go > backward, between .31 and upstream... Found something odd, perhaps Rex can test? I tested this on my old 2.4 and it worked well. Jocke From dd0f213d51437bb48ff42c1c0e690f243266c133 Mon Sep 17 00:00:00 2001 From: Joakim Tjernlund Date: Fri, 2 Oct 2009 14:59:21 +0200 Subject: [PATCH] powerpc, 8xx: DTLB Error must check for more errors. DataTLBError do only check for store op's. I think this is too narrow. Protection and and no translation errors needs to be checked too. --- arch/powerpc/kernel/head_8xx.S | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) -- 1.6.4.4 diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S index 52ff8c5..ef8a5ce 100644 --- a/arch/powerpc/kernel/head_8xx.S +++ b/arch/powerpc/kernel/head_8xx.S @@ -472,7 +472,7 @@ DataTLBError: /* First, make sure this was a store operation. */ mfspr r10, SPRN_DSISR - andis. r11, r10, 0x0200 /* If set, indicates store op */ + andis. r11, r10, 0x4a00 /* If set, store, protection or no trans. */ beq 2f /* The EA of a data TLB miss is automatically stored in the MD_EPN