From owner-freebsd-arm@FreeBSD.ORG Sat Sep 3 19:40:09 2011 Return-Path: Delivered-To: freebsd-arm@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B394D1065670 for ; Sat, 3 Sep 2011 19:40:09 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 991458FC13 for ; Sat, 3 Sep 2011 19:40:09 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p83Je91B004191 for ; Sat, 3 Sep 2011 19:40:09 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p83Je9fo004190; Sat, 3 Sep 2011 19:40:09 GMT (envelope-from gnats) Date: Sat, 3 Sep 2011 19:40:09 GMT Message-Id: <201109031940.p83Je9fo004190@freefall.freebsd.org> To: freebsd-arm@FreeBSD.org From: Mark Tinguely Cc: Subject: Re: arm/160431: [patch] Disable interrupts during busdma cache sync operations. X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Mark Tinguely List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Sep 2011 19:40:09 -0000 The following reply was made to PR arm/160431; it has been noted by GNATS. From: Mark Tinguely To: Ian Lepore Cc: FreeBSD-gnats-submit@FreeBSD.org Subject: Re: arm/160431: [patch] Disable interrupts during busdma cache sync operations. Date: Sat, 03 Sep 2011 14:05:32 -0500 On 9/3/2011 12:19 PM, Ian Lepore wrote: >> Number: 160431 >> Category: arm >> Synopsis: [patch] Disable interrupts during busdma cache sync operations. >> Confidential: no >> Severity: critical >> Priority: high >> Responsible: freebsd-arm >> State: open >> Quarter: >> Keywords: >> Date-Required: >> Class: sw-bug >> Submitter-Id: current-users >> Arrival-Date: Sat Sep 03 17:20:12 UTC 2011 >> Closed-Date: >> Last-Modified: >> Originator: Ian Lepore >> Release: FreeBSD 8.2-RC3 arm >> Organization: > none >> Environment: > FreeBSD dvb 8.2-RC3 FreeBSD 8.2-RC3 #49: Tue Feb 15 22:52:14 UTC 2011 root@revolution.hippie.lan:/usr/obj/arm/usr/src/sys/DVB arm > >> Description: > Data can be corrupted when an interrupt occurs while busdma_sync_buf() is > handling a buffer that partially overlaps a cache line. One scenario, seen > in the real world, was a small IO buffer allocated in the same cache line > as an instance of a struct intr_handler. The dma sync code copied the non-DMA > data (the intr_handler struct) to a temporary buffer prior to the cache sync, > then an interrupt occurs that results in setting the it_need flag in the > struct. When control returns to the dma sync code it finishes by copying > the saved partial cache line from the temporary buffer back to the > intr_handler struct, restoring the it_need flag to zero, and resulting in > a threaded interrupt handler not running as needed. > > Similar sequences can be imagined that would lead to corruption of either > the DMA'd data or non-DMA data sharing the same cache line, depending on the > timing of the interrupt, and I can't quite convince myself that the problem > only occurs in this partial-cacheline-overlap scenario. For example, what > happens if execution is in the middle of a cpu_dcache_wbinv_range() operation > and an interrupt leads to a context switch wherein the pmap code decides to > call cpu_dcache_inv_range()? So to be conservatively safe, this patch > disables interrupts for the entire duration bus_dmamap_sync_buf(), not just > when partial cache lines are being handled. > >> How-To-Repeat: > It would be very difficult to set up a repeatable test of this condition. We > were lucky (!) enough to have it happen repeatably enough to diagnose. > >> Fix: > Problem was discovered in an 8.2 environment, but this diff is to -current. > > --- diff.tmp begins here --- > --- busdma_machdep.c.orig 2010-03-11 14:16:54.000000000 -0700 > +++ busdma_machdep.c 2011-09-03 10:15:16.000000000 -0600 > @@ -1091,6 +1091,14 @@ static void > bus_dmamap_sync_buf(void *buf, int len, bus_dmasync_op_t op) > { > char _tmp_cl[arm_dcache_align], _tmp_clend[arm_dcache_align]; > + uint32_t intr; > + > + /* Interrupts MUST be disabled when handling partial cacheline flush > + * and most likely should be disabled for all flushes. (I know for > + * certain interrupts can cause failures on partial flushes, and suspect > + * problems could also happen in other scenarios.) > + */ > + intr = intr_disable(); > > if ((op& BUS_DMASYNC_PREWRITE)&& !(op& BUS_DMASYNC_PREREAD)) { > cpu_dcache_wb_range((vm_offset_t)buf, len); > @@ -1129,6 +1137,8 @@ bus_dmamap_sync_buf(void *buf, int len, > arm_dcache_align - (((vm_offset_t)(buf) + len)& > arm_dcache_align_mask)); > } > + > + intr_restore(intr); > } > > static void > --- diff.tmp ends here --- > >> Release-Note: >> Audit-Trail: >> Unformatted: > _______________________________________________ > Which processor are you using (for my curiosity)? If this is easily reproducible, would you please put the interrupt disable/restore just around the BUS_DMASYNC_POSTREAD option? (for my curiosity again). Thank-you --Mark.