On Fri, 2016-04-22 at 14:12 +0100, Andre Przywara wrote:
Hi,
On 22/04/16 13:09, Hans de Goede wrote:
Hi,
On 22-04-16 13:46, Andre Przywara wrote:
Hi Hans,
thanks for the information and the heads up!
On 22/04/16 11:48, Hans de Goede wrote:
Hi,
On 22-04-16 11:32, Ian Campbell wrote:
On Fri, 2016-04-15 at 09:34 +0200, Hans de Goede wrote:
> > I wonder if what you are observing could be possibly > explained by > just > a usual data corruption problem? Which may be happening > when the DRAM > clock speed is set higher than this particular device is > able to > handle > in a reliable way. Inserting just one or more NOP > instructions > instead > of the barrier could possibly change some timings too. > > If this patch helps, then it's fine. But I wonder if it > is not merely > making the problem latent instead of fixing the root > cause? I do believe that this patch addresses a real problem and is not hiding some dram timing issues, I might be wrong about the write- buffer being the cause, it could simply be that the compiler is doing something bad (despite the accesses being marked as volatile) and that the DSB stops the compiler from optimizing things too much.
I have a _very_ vague memory of seeing something not disimilar to this (apparent write buffer interactions with MMU disabled) in the early days of Xen development, but that was probably on models and so may not have been representative of the intended behaviour of eventual silicon.
It might be interesting to have a look at the generated assembly and see if it differs in more or less than the addition of the single instruction and perhaps experiment with just a compiler barrier.
Andre, do you have any insights on this?
Agree on the compiler barrier, frankly I don't see how this should break with caches on or off unless the actual instruction order is wrong or the compiler optimized something away. Regardless of the write buffer the core should make sure the subsequent reads return the value written before - especially if we are talking UP here.
"the core should make sure the subsequent reads return the value written before" that is exactly the problem, we are writing 2 different values to so DRAM_BASE and DRAM_BASE + 512MiB, then read them both back and compare them, expecting them to be the same (both reads returning the last written value) if the ramsize is 512MiB (this is used in several places in the dram controller code to auto-config number of rows, columns, etc.).
But the core seems to just return the last written value, rather then actually going out to the RAM and reading it from there, which results in the function always returning false (i.o.w. it claims no DRAM phys address wraparound is happening at 512MiB).
Oh, right, I missed that part, sorry. So this is about physical aliasing? The DRAM controller has only n address lines connected, and changing a line >n shouldn't make a difference, right?
Correct, it's a technique used to try and size the DRAM by determing n by observation of the aliasing patterns.
And the write succeeds and does trigger an asynchronous abort?
^n't
So I did a quick poll around the office and people say that "dsb" is the right thing to do here (with MMU off). As this is backed by practical experience, I'd just say: good to go!
Patch therefore:
Acked-by: Ian Campbell ijc@hellion.org.uk
Ian.