All of lore.kernel.org
 help / color / mirror / Atom feed
From: Aneesh V <aneesh@ti.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] ARM926ejs: Add routines to invalidate D-Cache
Date: Fri, 05 Aug 2011 17:21:47 +0530	[thread overview]
Message-ID: <4E3BD953.8050502@ti.com> (raw)
In-Reply-To: <4E3BCDEB.8090106@aribaud.net>

Hi Albert,

On Friday 05 August 2011 04:33 PM, Albert ARIBAUD wrote:
> Hi Aneesh,
>
> On 05/08/2011 12:47, Aneesh V wrote:
>> Hi Eric,
>>
>> On Friday 05 August 2011 04:03 PM, Hong Xu wrote:
>>> Hi Aneesh,
>> [snip ..]
>>>>
>>>> IMHO, Hong's approach is correct. If the buffer that is invalidated is
>>>> not aligned to cache-line, one cache-line at the respective boundary
>>>> may have to be flushed to make sure the invalidation doesn't affect
>>>> somebody else's memory.
>>>>
>>>> The solution is for drivers to ensure that any buffer that needs to be
>>>> invalidated is aligned to cache-line boundary at both ends. The above
>>>> approach puts this onus on the driver. I have documented the alignment
>>>> requirement in my recent patch series for fixing arm cache problems.
>>>
>>> I have not noticed the patch series. ;-)
>>> If we put the alignment burden to the driver, I'm afraid many drivers
>>> which make use of something like a DMA controller have to modify the
>>> code heavily. This sounds not good. :)
>>
>> We have a fundamental problem when it comes to invalidating an
>> un-aligned buffer. Either you flush the boundary lines and corrupt your
>> buffer at boundaries OR you invalidate without flushing and corrupt
>> memory around your buffer. Both are not good! The only real solution is
>> to have aligned buffers, if you want to have D-cache enabled and do DMA
>> at the same time.
>
> Plus, there should not be *heavy* modifications; DMA engines tend to use
> essentially two types of memory-resident objects: data buffers and
> buffer descriptors. There's only a small handful of places in the driver
> code to look at to find where these objects are allocated and how.
>
> So I stand by my opinion: since the cache invalidation routine should
> only be called with cache-aligned objects, there is no requirement to
> flush the first (resp. last) cache line in case of unaligned start
> (resp.stop), and I don't want cache operations performed when they are
> not required.

IMHO, flushing is better, because the person who commits the
mistake of invalidating the un-aligned buffer is the one who is
affected and is likely to fix the issue soon. If we didn't flush, the
resulting corruption will cause totally random errors that will be hard
to debug. Doing an extra flush operation for a maximum of 2 lines
doesn't cost us anything. This is the approach followed by the kernel
too.

br,
Aneesh

  parent reply	other threads:[~2011-08-05 11:51 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-05  4:44 [U-Boot] [PATCH v2] ARM926ejs: Add routines to invalidate D-Cache Hong Xu
2011-08-05  5:11 ` Reinhard Meyer
2011-08-05  6:17   ` Hong Xu
2011-08-05  6:22     ` Albert ARIBAUD
2011-08-05  6:13 ` Albert ARIBAUD
2011-08-05  6:38   ` Hong Xu
2011-08-05  6:46     ` Albert ARIBAUD
2011-08-05  7:02       ` Hong Xu
2011-08-05  7:10       ` Aneesh V
2011-08-05  9:20         ` Albert ARIBAUD
2011-08-05  9:56           ` Aneesh V
2011-08-05 10:33         ` Hong Xu
2011-08-05 10:47           ` Aneesh V
2011-08-05 11:03             ` Albert ARIBAUD
2011-08-05 11:23               ` Reinhard Meyer
2011-08-05 11:26                 ` Albert ARIBAUD
2011-08-05 11:51               ` Aneesh V [this message]
2011-08-05 13:17                 ` Albert ARIBAUD
2011-08-05 14:59                   ` Aneesh V
2011-08-07  6:55                     ` Albert ARIBAUD
2011-08-08  8:24                       ` Aneesh V
2011-08-08  9:39                         ` Albert ARIBAUD
2011-08-08  9:51                           ` Aneesh V
2011-08-08  9:59                           ` Reinhard Meyer
2011-08-08 10:12                             ` Aneesh V
2011-08-08 10:25                               ` Reinhard Meyer
2011-08-08 10:27                                 ` Aneesh V
2011-08-08 11:05                                   ` Albert ARIBAUD
2011-08-05 23:04                 ` J. William Campbell
2011-08-07  7:07                   ` Albert ARIBAUD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E3BD953.8050502@ti.com \
    --to=aneesh@ti.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.