From owner-freebsd-usb@FreeBSD.ORG Mon Jun 29 13:17:29 2009 Return-Path: Delivered-To: freebsd-usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 100851065672; Mon, 29 Jun 2009 13:17:29 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe10.swipnet.se [212.247.155.33]) by mx1.freebsd.org (Postfix) with ESMTP id 054D58FC1B; Mon, 29 Jun 2009 13:17:27 +0000 (UTC) (envelope-from hselasky@c2i.net) X-Cloudmark-Score: 0.000000 [] X-Cloudmark-Analysis: v=1.0 c=1 a=Hrwt8fWgTlIA:10 a=gg2W7PyvkLb8p4ie143lBA==:17 a=Mm9x50jhgXV4_Hr8jdcA:9 a=oA1dRUiy9LCzW5VhFsIA:7 a=Uf1fOaIE_9Flh2HDhU2pLKyBf9gA:4 Received: from [194.248.135.20] (account mc467741@c2i.net HELO laptop.adsl.tele2.no) by mailfe10.swip.net (CommuniGate Pro SMTP 5.2.13) with ESMTPA id 1098585684; Mon, 29 Jun 2009 15:17:26 +0200 From: Hans Petter Selasky To: Piotr =?iso-8859-2?q?Zi=EAcik?= , freebsd-arm@freebsd.org Date: Mon, 29 Jun 2009 15:16:56 +0200 User-Agent: KMail/1.11.4 (FreeBSD/8.0-CURRENT; KDE/4.2.4; i386; ; ) References: <200906231035.43096.kosmo@semihalf.com> In-Reply-To: <200906231035.43096.kosmo@semihalf.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-2" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200906291516.58725.hselasky@c2i.net> Cc: Rafal Jaworowski , thompsa@freebsd.org, freebsd-usb@freebsd.org Subject: Re: CPU Cache and busdma usage in USB X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Jun 2009 13:17:29 -0000 On Tuesday 23 June 2009 10:35:42 Piotr Zi=EAcik wrote: > --- a/sys/dev/usb/usb_busdma.c > +++ b/sys/dev/usb/usb_busdma.c > @@ -658,8 +658,7 @@ usb_pc_cpu_invalidate(struct usb_page_cache *pc) > /* nothing has been loaded into this page cache! */ > return; > } > - bus_dmamap_sync(pc->tag, pc->map, > - BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD); > + bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREREAD); > } > > /*----------------------------------------------------------------------= =2D- >* @@ -672,8 +671,7 @@ usb_pc_cpu_flush(struct usb_page_cache *pc) > /* nothing has been loaded into this page cache! */ > return; > } > - bus_dmamap_sync(pc->tag, pc->map, > - BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD); > + bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREWRITE); > } Hi, Let's restart the discussion with the initial patch. You want to change the flags passed to bus_dmamap_sync() so that the=20 flush/invalidate mapping gets right. I understand why your patch makes it=20 work. That's not the problem. In "src/sys/arm/arm/busdma_machdep.c" there is a function called=20 "_bus_dmamap_sync_bp()". If you look at that function you see that it only= =20 triggers on the "BUS_DMASYNC_PREWRITE" and "BUS_DMASYNC_POSTREAD" flags. Af= ter=20 your patching only the PREXXXX flags are used, so if bouce pages are used o= n=20 ARM and x86 and amd64 +++, then only BUS_DMASYNC_PREWRITE will do anything.= =20 This indicates that your patch is not fully correct. Grepping through the source code for ARM, I found a line like this: /*XXX*/ arm9_dcache_wbinv_range, /* dcache_inv_range */ which is not correct. If we are only invalidating, then it is not correct t= o=20 do a flush first. Summed up: 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]; if ((op & BUS_DMASYNC_PREWRITE) && !(op & BUS_DMASYNC_PREREAD)) { cpu_dcache_wb_range((vm_offset_t)buf, len); cpu_l2cache_wb_range((vm_offset_t)buf, len); } if (op & BUS_DMASYNC_PREREAD) { if (!(op & BUS_DMASYNC_PREWRITE) && ((((vm_offset_t)(buf) | len) & arm_dcache_align_mask) = =3D=3D=20 0)) { cpu_dcache_inv_range((vm_offset_t)buf, len); cpu_l2cache_inv_range((vm_offset_t)buf, len); } else { Because the USB code specifies both PREREAD and PREWRITE we end up in the=20 following case, which is not implemented correctly. The function name=20 indicates write back first, then invalidate, but when looking at the=20 implementation: arm8_cache_purgeID, /* idcache_wbinv_all */ (void *)arm8_cache_purgeID, /* idcache_wbinv_range */ You see that it only performs purge and no prior flush. This is what needs= =20 fixing! If semihalf could go through the "arm/arm/cpufunc.c" file and fix=20 those flush and invalidate functions then many people would become happy! Again, it is not a problem in USB firstly, it is a problem in "arm/xxx". cpu_dcache_wbinv_range((vm_offset_t)buf, len); cpu_l2cache_wbinv_range((vm_offset_t)buf, len); } } 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); } 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,=20 (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)); } } =2D-HPS