From owner-svn-src-all@FreeBSD.ORG Mon Jun 8 16:13:24 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D479D1065670; Mon, 8 Jun 2009 16:13:24 +0000 (UTC) (envelope-from raj@semihalf.com) Received: from smtp.semihalf.com (smtp.semihalf.com [213.17.239.109]) by mx1.freebsd.org (Postfix) with ESMTP id 77A9A8FC16; Mon, 8 Jun 2009 16:13:24 +0000 (UTC) (envelope-from raj@semihalf.com) Received: from [10.0.0.34] (cardhu.semihalf.com [213.17.239.108]) by smtp.semihalf.com (Postfix) with ESMTPSA id C4F61C3AB7; Mon, 8 Jun 2009 17:54:57 +0200 (CEST) Message-Id: <9CB8E78B-BED8-4995-9823-5CEE75A4F52F@semihalf.com> From: Rafal Jaworowski To: Stanislav Sedov In-Reply-To: <20090608165646.95bb577e.stas@FreeBSD.org> Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit Mime-Version: 1.0 (Apple Message framework v935.3) Date: Mon, 8 Jun 2009 17:56:51 +0200 References: <200906081215.n58CFdOl029049@svn.freebsd.org> <20090608165646.95bb577e.stas@FreeBSD.org> X-Mailer: Apple Mail (2.935.3) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, =?ISO-8859-2?Q?Piotr_Zi=EAcik?= Subject: Re: svn commit: r193712 - head/sys/arm/arm X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Jun 2009 16:13:26 -0000 On 2009-06-08, at 14:56, Stanislav Sedov wrote: > On Mon, 8 Jun 2009 12:15:39 +0000 (UTC) > Rafal Jaworowski mentioned: > >> Author: raj >> Date: Mon Jun 8 12:15:39 2009 >> New Revision: 193712 >> URL: http://svn.freebsd.org/changeset/base/193712 >> >> Log: >> Invalidate cache in pmap_remove_all() on ARM. >> >> When pages are removed from virtual address space by calling >> pmap_remove_all() >> CPU caches were not invalidated, which led to read corruption when >> another >> page got mapped at this same virtual address at later time (the >> CPU was >> retrieving stale contents). >> >> Submitted by: Piotr Ziecik >> Obtained from: Semihalf >> >> Modified: >> head/sys/arm/arm/pmap.c >> >> Modified: head/sys/arm/arm/pmap.c >> = >> = >> = >> = >> = >> = >> = >> = >> = >> ===================================================================== >> --- head/sys/arm/arm/pmap.c Mon Jun 8 12:10:42 2009 (r193711) >> +++ head/sys/arm/arm/pmap.c Mon Jun 8 12:15:39 2009 (r193712) >> @@ -3124,7 +3124,19 @@ pmap_remove_all(vm_page_t m) >> if (flush == FALSE && (pv->pv_pmap == curpm || >> pv->pv_pmap == pmap_kernel())) >> flush = TRUE; >> + >> PMAP_LOCK(pv->pv_pmap); >> + /* >> + * Cached contents were written-back in pmap_remove_write(), >> + * but we still have to invalidate the cache entry to make >> + * sure stale data are not retrieved when another page will be >> + * mapped under this virtual address. >> + */ >> + if (pmap_is_current(pv->pv_pmap)) { >> + cpu_dcache_inv_range(pv->pv_va, PAGE_SIZE); >> + cpu_l2cache_inv_range(pv->pv_va, PAGE_SIZE); >> + } >> + > > Hi, Rafal! > > What about calling the pmap_dcache_wb_range function for each > mapping in > the cycle instead of calling cpu_XXX_inv_range functions directly? > I think > something like this would do the trick: > % pmap_dcache_wb_range(pv->pv_pmap, pv->pv_va, PAGE_SIZE, FALSE, > % (pv->pv_flags & PVF_WRITE) == 0) > > This will also take care of the writeback cache. I don't know if it > is really > needed, though. Do you see anything wrong with calling cpu_xxx_inv_range directly? Writing back (if required) was already performed by pmap_remove_write(), and what we only need at this point is invalidation. pmap_dcache_wb_range would also eventually call cpu_XXX_inv_range if given a proper combination of flags (BTW: the do_inv flag in your example should be TRUE for our context to work), so it wouldn't be any simpler. I'd rather prefer doing explicitly what is needed without extra wrapping. Rafal