Re: [Qemu-arm] [PATCH] tests/microbit-test: Add tests for nRF51 NVMC

[Peter Maydell]

On Mon, 28 Jan 2019 at 19:01, Peter Maydell <address@hidden> wrote:
> This assertion fails on PPC64 hosts:
> ERROR:/home/pm215/qemu/tests/microbit-test.c:181:fill_and_erase:
> assertion failed (qtest_readl(qts, base + i * 4) == i): (16777216 ==
> 1)
>
> I suspect an endianness issue, given that 16777216 is 0x0100_0000.
> The test looks OK, though -- a word write ought to come back
> the same on a word read -- so maybe it's in the implementation?

The problem here is that the read path goes directly to the
underlying host memory, but the write path goes via the
flash_write() function, and the two disagree about the
representation of the underlying storage.

I think that declaring s->storage to be a uint32_t* is a mistake --
the underlying RAM needs to be treated as a byte array, not as an
array of host-endian-order 32-bit words (because the direct-to-RAM
path effectively treats it as a byte array). Then, since the
flash_write() function is part of a DEVICE_LITTLE_ENDIAN
MemoryRegionOps, its 'value' argument is a 32-bit LE value, and
it should do the modification of s->storage via something like
   oldval = ldl_le_p(s->storage + offset);
   oldval &= value;
   stl_le_p(s->storage + offset, oldval);

Another reference to s->storage also simplifies when we fix its
type: s->storage + value / 4 becomes just s->storage + value.

NB: untested!

thanks
-- PMM