Date: Sat, 3 Sep 2011 19:40:09 GMT From: Mark Tinguely <marktinguely@gmail.com> To: freebsd-arm@FreeBSD.org Subject: Re: arm/160431: [patch] Disable interrupts during busdma cache sync operations. Message-ID: <201109031940.p83Je9fo004190@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR arm/160431; it has been noted by GNATS.
From: Mark Tinguely <marktinguely@gmail.com>
To: Ian Lepore <freebsd@damnhippie.dyndns.org>
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<freebsd@damnhippie.dyndns.org>
>> 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201109031940.p83Je9fo004190>