On Tuesday, October 13, 2015 at 03:04:44 AM, Thomas Chou wrote: > Hi Marek,
Hi! > On 10/12/2015 09:29 PM, Marek Vasut wrote: > > On Monday, October 12, 2015 at 03:12:18 PM, Thomas Chou wrote: > >> Hi Marek, > >> > >> On 10/12/2015 06:30 PM, Marek Vasut wrote: > >>> There are also DEFINE_CACHE_ALIGN_BUFFER() and > >>> ALLOC_CACHE_ALIGN_BUFFER() macros which can be used to allocate such > >>> stuff on stack. And you sometimes do want to allocate things on stack > >>> instead of using malloc(). > >> > >> Thanks for sharing this. > >> > >>> Sometimes you might want to allocate DMA buffers on stack, for example > >>> if you don't have mallocator running yet or if it's more convenient > >>> for some reason. So forcing everyone to allocate DMA buffers using > >>> malloc is not gonna slide I'm afraid. > >> > >> The same rule can be applied to buffer allocated on stack, with the > >> macro you mentioned above. In all, cache line aware allocation on heap > >> or on stack must be used for DMA buffer. > > > > That's correct, they must be used. But sadly, this is not yet the case in > > all the drivers, which we need to rectify. And how best to rectify this > > than to scream when someone does such a thing, right ? > > Given that there are not so many drivers using DMA in u-boot, as > compared to Linux. I would suggest we can walk through and rectify the > allocation of DMA buffers. That's what we're pretty much trying to do -- fix all the DMA-using drivers to behave correctly. > >>> The cache flush ops is the best place to scream death and murder if > >>> someone tries such unaligned cache operation, so maybe you should even > >>> do a printf() there to weed such crappy drivers out for the 2016.01 > >>> release. > >>> > >>> I agree it's the responsibility of the driver, so if the driver doesn't > >>> do things right, it's a bug and the behavior of cache ops is undefined, > >>> which might as well be that we do the safer thing here and flush > >>> nothing. > >> > >> It won't be safer to flush nothing. Sooner or later the cache will be > >> flushed due to data access, even if the cache flush ops is skip. > > > > That is bad bad bad, that's even nastier. We really need to fix the > > drivers, not paper over it in the cache ops. > > I know how bad it is, with over 35 years work with DMA. grin.. I can feel your pain here ;-/ > >> To solve problem like this, the only solution is to enforce the rule to > >> allocate DMA buffer. It is wrong to skip the flush. > > > > I absolutelly agree we need aligned allocations for DMA memory areas. > > But, we also shouldn't hide bugs. And I believe aligning the incorrect > > arguments to cache functions is not the way to go. We should check the > > arguments and if someone tries an unaligned cache op, we should scream. > > What do you think? > > > > btw. I think you won't get way too many cache warnings nowadays and we > > can fix those few remaining way before the 2016.01 is out. > > I would suggest the "cache alignment check and skip" be removed from > cache flush ops, and say out the DMA buffer allocation rule loudly in > README, and enforce it by guardianship. What exactly do you envision by this "guardianship" ? > Please allow me to restate the reasons, > > 1. The cache flush ops are commonly used. Please refer to the "Cache and > TLB Flushing Under Linux" doc, linux/Documentation/cachetlb.txt. > Violating the defined interface is much worse than violating coding > style. It will certainly impact the portability of u-boot. And might > introduce more bug than resolve. I agree with this one. > 2. We all agree that enforcing DMA buffer allocation to cache aligned is > the only real solution. Adding such "check and skip" to cache flush ops > cannot prevent the flush or solve the problem. We should probably check-scream-skip here. > 3. Though the flush size of block device are usually aligned, the size > of packet are not. Asking the packet drivers to adjust the flush size > does not make sense. It is the job of cache flush ops. The debug probe > should not override the original purpose. It should be spelled for > common understanding. The socket buffer(s) should be aligned, so network packets should be fine. > It is free to your consideration. As it is free and open software. :) > > Best regards, > Thomas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot