Date: Sat, 21 Apr 2012 16:30:21 GMT From: Ian Lepore <freebsd@damnhippie.dyndns.org> To: freebsd-arm@FreeBSD.org Subject: Re: arm/160431: [busdma] [patch] Disable interrupts during busdma cache sync operations. Message-ID: <201204211630.q3LGULkU086330@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: Ian Lepore <freebsd@damnhippie.dyndns.org> To: bug-followup@FreeBSD.org Cc: Subject: Re: arm/160431: [busdma] [patch] Disable interrupts during busdma cache sync operations. Date: Sat, 21 Apr 2012 10:22:47 -0600 --=-qM25b7nGVVUxHWsdaUrf Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Arrgh! One more time, with the actual correct patch attached (and this time validated correctly by building kernel rather than world, doh!). --=-qM25b7nGVVUxHWsdaUrf Content-Description: Content-Disposition: inline; filename="arm_busdma.diff" Content-Type: text/x-patch; name="arm_busdma.diff"; charset="us-ascii" Content-Transfer-Encoding: 7bit Index: sys/arm/arm/busdma_machdep.c =================================================================== --- sys/arm/arm/busdma_machdep.c (revision 234543) +++ sys/arm/arm/busdma_machdep.c (working copy) @@ -1090,6 +1090,8 @@ static void bus_dmamap_sync_buf(void *buf, int len, bus_dmasync_op_t op) { + uint32_t intr; + int partial; char _tmp_cl[arm_dcache_align], _tmp_clend[arm_dcache_align]; if ((op & BUS_DMASYNC_PREWRITE) && !(op & BUS_DMASYNC_PREREAD)) { @@ -1106,28 +1108,45 @@ cpu_l2cache_wbinv_range((vm_offset_t)buf, len); } } + /* + * Interrupts must be disabled while handling a partial cacheline flush, + * otherwise the interrupt handling code could modify data in the + * non-DMA part of a cacheline while we have it stashed away in the + * temporary stack buffer, then we end up restoring the stale value. + * As unlikely as this seems, it has been observed in the real world. + */ if (op & BUS_DMASYNC_POSTREAD) { - if ((vm_offset_t)buf & arm_dcache_align_mask) { - memcpy(_tmp_cl, (void *)((vm_offset_t)buf & ~ - arm_dcache_align_mask), - (vm_offset_t)buf & arm_dcache_align_mask); + partial = (((vm_offset_t)buf) | len) & arm_dcache_align_mask; + if (partial) { + intr = intr_disable(); + if ((vm_offset_t)buf & arm_dcache_align_mask) { + memcpy(_tmp_cl, (void *)((vm_offset_t)buf & + ~arm_dcache_align_mask), + (vm_offset_t)buf & arm_dcache_align_mask); + } + if (((vm_offset_t)buf + len) & arm_dcache_align_mask) { + memcpy(_tmp_clend, + (void *)((vm_offset_t)buf + len), + arm_dcache_align - + (((vm_offset_t)(buf) + len) & + arm_dcache_align_mask)); + } } - if (((vm_offset_t)buf + len) & arm_dcache_align_mask) { - memcpy(_tmp_clend, (void *)((vm_offset_t)buf + len), - arm_dcache_align - (((vm_offset_t)(buf) + len) & - arm_dcache_align_mask)); - } cpu_dcache_inv_range((vm_offset_t)buf, len); cpu_l2cache_inv_range((vm_offset_t)buf, len); - - if ((vm_offset_t)buf & arm_dcache_align_mask) - memcpy((void *)((vm_offset_t)buf & - ~arm_dcache_align_mask), _tmp_cl, - (vm_offset_t)buf & arm_dcache_align_mask); - if (((vm_offset_t)buf + len) & arm_dcache_align_mask) - memcpy((void *)((vm_offset_t)buf + len), _tmp_clend, - arm_dcache_align - (((vm_offset_t)(buf) + len) & - arm_dcache_align_mask)); + if (partial) { + if ((vm_offset_t)buf & arm_dcache_align_mask) + memcpy((void *)((vm_offset_t)buf & + ~arm_dcache_align_mask), _tmp_cl, + (vm_offset_t)buf & arm_dcache_align_mask); + if (((vm_offset_t)buf + len) & arm_dcache_align_mask) + memcpy((void *)((vm_offset_t)buf + len), + _tmp_clend, + arm_dcache_align - + (((vm_offset_t)(buf) + len) & + arm_dcache_align_mask)); + intr_restore(intr); + } } } --=-qM25b7nGVVUxHWsdaUrf--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204211630.q3LGULkU086330>