[U-Boot] Cache alignment warnings on Tegra (ARM)

[Thierry Reding]

On Sat, Sep 15, 2012 at 07:45:30PM -0700, Simon Glass wrote:
> Hi,
> 
> On Sat, Sep 15, 2012 at 1:41 PM, Thierry Reding
> <thierry.reding at avionic-design.de> wrote:
> > On Sat, Sep 15, 2012 at 10:11:54PM +0200, Marek Vasut wrote:
> >> Dear Thierry Reding,
> >>
> >> > On Fri, Sep 14, 2012 at 08:53:32AM -0700, Simon Glass wrote:
> >> > > Hi,
> >> > >
> >> > > On Wed, Sep 12, 2012 at 4:42 PM, Marek Vasut <marex at denx.de> wrote:
> >> > > > Dear Stephen Warren,
> >> > > >
> >> > > >> On 09/12/2012 04:38 PM, Marek Vasut wrote:
> >> > > >> > Dear Stephen Warren,
> >> > > >> >
> >> > > >> >> On 09/12/2012 10:19 AM, Tom Warren wrote:
> >> > > >> >>> Folks,
> >> > > >> >>>
> >> > > >> >>> Stephen Warren has posted an internal bug regarding the cache
> >> > > >> >>> alignment 'warnings' seen on Tegra20 boards when accessing MMC.
> >> > > >> >>> Here's the gist:
> >> > > >> >>>
> >> > > >> >>> Executing "mmc dev 0" still yields cache warnings:
> >> > > >> >>>
> >> > > >> >>> Tegra20 (Harmony) # mmc dev 0
> >> > > >> >>> ERROR: v7_dcache_inval_range- stop address is not aligned-
> >> > > >> >>> 0x3fb69908 mmc0 is current device
> >> > > >> >>
> >> > > >> >> ...
> >> > > >> >>
> >> > > >> >>> There have been patches in the past (IIRC) that have tried to
> >> > > >> >>> ensure all callers (FS, MMC driver, USB driver, etc.) force their
> >> > > >> >>> buffers to the appropriate alignment, but I don't know that we
> >> > > >> >>> can ever correct every instance, now or in the future.
> >> > > >> >>>
> >> > > >> >>> Can we start a discussion about what we can do about this warning?
> >> > > >> >>> Adding an appropriate #ifdef
> >> > > >> >>> (CONFIG_SYS_NO_CACHE_ALIGNMENT_WARNINGS, etc.) where Stephen put
> >> > > >> >>> his #if 0's would be one approach, or changing the printf() to a
> >> > > >> >>> debug(), perhaps. As far as I can tell, these alignment 'errors'
> >> > > >> >>> don't seem to produce bad data in the transfer.
> >> > > >> >>
> >> > > >> >> I don't think simply turning off the warning is the correct
> >> > > >> >> approach; I believe they represent real problems that can in fact
> >> > > >> >> cause data corruption. I don't believe we have any choice other
> >> > > >> >> than to fully solve the root-cause.
> >> > >
> >> > > Yes I agree, and I think it is pretty close - certainly much better
> >> > > than it used to be. The good thing about them being annoying is that
> >> > > they will likely get fixed :-)
> >> >
> >> > I think I traced this to the copying of CSD a while back. The problem is
> >> > that the transferred buffer is 8 bytes, so there's no way to make it
> >> > aligned properly. Unfortunately the entailing discussion did not yield a
> >> > solution at the time.
> >>
> >> And how exactly does the MMC bounce buffer not help here?
> >
> > The problem solved by the bounce buffer is that it is properly cache-
> > line aligned. However the issue here is not that the buffer is not
> > properly aligned but rather that the transfer is too small.
> >
> > When the MMC core transfers the SCR, it requests 8 bytes. The buffer
> > used to store these 8 bytes is properly allocated. However, those 8
> > bytes eventually end up as the size of the range that is to be
> > invalidated. This is the reason for the warning. Address x of the buffer
> > is cache-line aligned, but x + 8 is never properly aligned and therefore
> > the code complains.
> 
> Would it be too dreadful to define a minimum MMC transfer size, and
> set that to the cache line size?

I did try setting the data size to the cache line size back then, but
that resulted in an error. After that I gave up. I think what we really
need to do is separate the invalidation size from the transfer size in
order to properly handle these situations. Or alternatively pass an
additional buffer size so the code knows how much needs to be
invalidated. AFAICT this is the only location where this actually
happens. All other transfers are typically block sized so they'll be a
multiple of the cache line anyway.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120916/82a6b1e2/attachment.pgp>