On Sunday, October 11, 2015 at 02:38:35 AM, Thomas Chou wrote: > Hi Marek,
Hi, > On 10/11/2015 02:18 AM, Marek Vasut wrote: > > Then you'd also need means to allocate variables to aligned memory > > location to prevent invalid cache flush. (Linux does this with it's DMA > > API). We are much simpler and thus this abstraction is still not > > available. I wonder if the overhead of DMA API would be high or not for > > U-Boot. > > I see most people use memalign(ARCH_DMA_MINALIGN, len) in u-boot to > allocate DMA buffers so that they are cache aligned. That and include/memalign.h , which contains macros that are used to align variables. > > It is even worse if the cache flush operators permit incorrect cache > > flushes or invalidations. Like I mentioned before, this can lead to hard > > to debug problems when using DMA (at least on ARM). > > I would suggest debug check should be left as for debug only. The > definition of common functions should be kept as it is more important > than coding style. Uh yes, that's what arm926 cache functions do, they're debug only. > I debugged DMA issues a lot in the past until I realized the importance > of aligned buffers. So there should be a developer's guideline. For what exactly? > But it is even much more difficult when something you believed does not > work as expected, what is taken as common sense. It will trap a lot of > developers when they called your flush cache functions but was skipped > just because, eg, the end of packets are not aligned which is usually > the case. This is good, it should bite them, because this is a bug. If, on the other hand, you will paper over such bugs by adding crap to the cache ops, there will be even worse bugs coming for you, like variables which are sitting in the same cacheline as your unaligned buffer that you want to invalidate or flush will possibly get trashed by such cache operation. Consider this: cacheline 0: [ variable A ; buffer B ......... ] cacheline 1: [ buffer B ......... ; Empty .... ] Now you do the following: 1) Variable A = 0; 2) Flush buffer B (which is unaligned, so flush cacheline 0 and 1) 3) Start DMA to buffer B 4) Variable A = 1; 5) Check if DMA finished, it did 6) Invalidate buffer B ... oh, but it's unaligned, let's invalidate everything around it, which is cacheline 0 and 1. 7) What is the value of variable A ? Oh, it's fetched from memory and it's 0 there, even though we did set it to 1 ... > I would suggest that, with the best of my knowledge, please change the > range check to a debug probe, and restore the cache flush functions to > the common definition. See above, does my example make it clear why we should never ever hide bugs in the cache ops code ? Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
