public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] ARMv7 MMU shareability issue
Date: Wed, 9 Dec 2015 15:09:13 +0100	[thread overview]
Message-ID: <201512091509.13227.marex@denx.de> (raw)
In-Reply-To: <5668264B.6050301@denx.de>

On Wednesday, December 09, 2015 at 02:02:03 PM, Stefan Roese wrote:
> Hi All!
> 
> On 09.12.2015 09:09, Albert ARIBAUD wrote:
> > On Tue,  8 Dec 2015 14:43:42 +0100, Marek Vasut <marex@denx.de> wrote:
> >> The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this macro
> >> is set, it configures TTBR0 register. This register must be configured
> >> for the cache on ARMv7 to operate correctly.
> >> 
> >> The problem is that noone actually sets the CONFIG_ARMV7 macro and thus
> >> the TTBR0 is not configured at all. On SoCFPGA, this produces all sorts
> >> of minor issues which are hard to replicate, for example certain USB
> >> sticks are not detected or QSPI NOR sometimes fails to write pages
> >> completely.
> >> 
> >> The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one.
> >> This is correct because the code which added the test(s) for
> >> CONFIG_ARMV7 was added shortly after CONFIG_ARMV7 was replaced by
> >> CONFIG_CPU_V7 and this code was not adjusted correctly to reflect that
> >> change.
> > 
> > Note:
> > 
> > As discussed with Marek on IRC, this patch enables what is supposed to
> > be the correct MMU settings for ARMv7, which causes a sharp Ethernet
> > performance drop (40%) but also a strong general memory access
> > performance hit (a copy of 4 MB is almost instantaneous without the
> > patch and takes 2-3 seconds with it).
> 
> I've tested Marek's patch on my current Armada XP platform (also
> ARMv7). And also noticed a performance drop by about 30-40%.
> The dhrystone command can be used for this testing as well
> (CONFIG_CMD_DHRYSTONE).
> 
> So this is definitely a NAK from me to this current patch.

This is a wrong approach, this patch just fixes another patch which was broken 
from the getgo and the TTBR0 reg was not programmed correctly due to that. This
patch must be applied, but what we need to decide is whether we need _another_
patch which removes the S-bit from the pagetable or how to deal with that part.

> It
> fixes the issue on SoCFPGA but affects other platforms in a
> negative way (i.MX6, Armada XP / 38x and perhaps others too).
> My feeling is, that SoCFPGA needs to get fixed in a way, without
> affecting the other platforms.
> 
> Marek, have you checked if the dcache is enabled at all on
> SoCFPGA with this patch applied? I remember that before my
> patch to really enable the dcache on SoCFPGA, the performance
> was so low. And I even could remove all caching functions
> from the ethernet driver (invalidate and flush). And the
> driver still worked without any issues.

If I do dcache off, the performance drops further, so I suspect the cache is
indeed enabled.

> > I would like to either fix the performance or come up with an
> > explanation for it before I pick this patch.
> 
> Yes. Please don't apply it quickly.
> 
> Thanks,
> Stefan

Best regards,
Marek Vasut

  reply	other threads:[~2015-12-09 14:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-08 13:43 [U-Boot] [PATCH] arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7 Marek Vasut
2015-12-09  8:09 ` [U-Boot] ARMv7 MMU shareability issue (was: [PATCH] arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7) Albert ARIBAUD
2015-12-09 13:02   ` [U-Boot] ARMv7 MMU shareability issue Stefan Roese
2015-12-09 14:09     ` Marek Vasut [this message]
2015-12-10  2:27       ` Tom Rini
2015-12-10  3:53         ` Marek Vasut
2015-12-14  7:25           ` Albert ARIBAUD
2015-12-14  7:48             ` Pavel Machek
2015-12-14 11:10               ` Marek Vasut
2015-12-14 11:26               ` 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=201512091509.13227.marex@denx.de \
    --to=marex@denx.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox