Date: Mon, 8 Jun 2009 17:56:51 +0200 From: Rafal Jaworowski <raj@semihalf.com> To: Stanislav Sedov <stas@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, =?ISO-8859-2?Q?Piotr_Zi=EAcik?= <kosmo@semihalf.com> Subject: Re: svn commit: r193712 - head/sys/arm/arm Message-ID: <9CB8E78B-BED8-4995-9823-5CEE75A4F52F@semihalf.com> In-Reply-To: <20090608165646.95bb577e.stas@FreeBSD.org> References: <200906081215.n58CFdOl029049@svn.freebsd.org> <20090608165646.95bb577e.stas@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2009-06-08, at 14:56, Stanislav Sedov wrote: > On Mon, 8 Jun 2009 12:15:39 +0000 (UTC) > Rafal Jaworowski <raj@FreeBSD.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9CB8E78B-BED8-4995-9823-5CEE75A4F52F>