[U-Boot] [PATCH] nios2: convert cache flush to use dm cpu data

[Thomas Chou]

Hi Marek,

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.

>>> 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..

>
>> 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. 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.

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.

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.

It is free to your consideration. As it is free and open software. :)

Best regards,
Thomas