From patchwork Tue Oct 6 08:06:48 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joakim Tjernlund X-Patchwork-Id: 35071 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 A6441B7E2F for ; Tue, 6 Oct 2009 19:11:36 +1100 (EST) Received: by ozlabs.org (Postfix) id 52A3CB7BB4; Tue, 6 Oct 2009 19:11:29 +1100 (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 7DA52B7BAC for ; Tue, 6 Oct 2009 19:11:28 +1100 (EST) Received: from sesr04.transmode.se (sesr04.transmode.se [192.168.201.15]) by gw1.transmode.se (Postfix) with ESMTP id D4429650001; Tue, 6 Oct 2009 10:11:22 +0200 (CEST) In-Reply-To: <1254793935.1959.1.camel@pasglop> References: <1254744999-3158-1-git-send-email-Joakim.Tjernlund@transmode.se> <20091005220420.GA27923@compile2.chatsunix.int.mrv.com> <1254782248.7122.49.camel@pasglop> <1254793935.1959.1.camel@pasglop> Subject: Re: [PATCH 0/6] PowerPc 8xx TLB/MMU fixes X-KeepSent: AEE03401:0C51FEE2-C1257647:002C29FE; type=4; name=$KeepSent To: Benjamin Herrenschmidt X-Mailer: Lotus Notes Release 8.5 December 05, 2008 Message-ID: From: Joakim Tjernlund Date: Tue, 6 Oct 2009 10:06:48 +0200 X-MIMETrack: Serialize by Router on sesr04/Transmode(Release 8.5 HF407|May 07, 2009) at 2009-10-06 10:11:23 MIME-Version: 1.0 Cc: Scott Wood , "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 06/10/2009 03:52:15: > > \ > > So how does this look? Does it change anything? > > It should as the previous way was way off :( > > > > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > > index c33c6de..08a392f 100644 > > --- a/arch/powerpc/mm/fault.c > > +++ b/arch/powerpc/mm/fault.c > > @@ -153,7 +153,7 @@ int __kprobes do_page_fault(struct pt_regs *regs, > unsigned long address, > > #ifdef DEBUG_DCBX > > const char *istr = NULL; > > > > - insn = *((unsigned long *)regs->nip); > > + __get_user(insn, (unsigned long __user *)regs->nip); > > No, use get_user() not __get_user() or if you use the later, also use > access_ok(), and test the result in case it errors (if it does, you > probably want to just goto bad access and SEGV). OK, lets see what this gives us: diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index c33c6de..1bf91d3 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -153,7 +153,8 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address, #ifdef DEBUG_DCBX const char *istr = NULL; - insn = *((unsigned long *)regs->nip); + insn = 0; + __get_user(insn, (unsigned long __user *)regs->nip); if (((insn >> (31-5)) & 0x3f) == 31) { if (((insn >> 1) & 0x3ff) == 1014) /* dcbz ? 0x3f6 */ istr = "dcbz"; @@ -171,27 +172,32 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address, dar = regs->gpr[rb]; if (ra) dar += regs->gpr[ra]; - if (dar != address && address != 0x00f0 && trap == 0x300) + if (dar != address && trap == 0x300) printk(KERN_CRIT "%s: address:%lx, dar:%lx!\n", istr, address, dar); if (!strcmp(istr, "dcbst") && is_write) { printk(KERN_CRIT "dcbst R%ld,R%ld = %lx as a store, fixing!\n", ra, rb, dar); is_write = 0; } - +#if 0 if (trap == 0x300 && address != dar) { __asm__ ("mtdar %0" : : "r" (dar)); return 0; } +#endif } } #endif if (address == 0x00f0 && trap == 0x300) { - pte_t *ptep; + //pte_t *ptep; /* This is from a dcbX or icbi insn gone bad, these * insn do not set DAR so we have to do it here instead */ - insn = *((unsigned long *)regs->nip); + if (get_user(insn, (unsigned long __user *)regs->nip)) { + printk(KERN_CRIT "get_user failed, NIP:%lx\n", + regs->nip); + goto bad_area_nosemaphore; + } ra = (insn >> (31-15)) & 0x1f; /* Reg RA */ rb = (insn >> (31-20)) & 0x1f; /* Reg RB */ @@ -206,7 +212,7 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address, trap, address, dar, error_code, istr); #endif address = dar; -#if 1 +#if 0 if (is_write && get_pteptr(mm, dar, &ptep, NULL)) { pte_t my_pte = *ptep; @@ -216,7 +222,7 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address, } } #else - return 0; + //return 0; #endif } }