On Tue, Jan 29, 2019 at 7:47 PM Peter Maydell <[email protected]> wrote: > On Mon, 28 Jan 2019 at 19:01, Peter Maydell <[email protected]> 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 for looking into this! I'll retest on a big-endian host and send a new revision. Stefan
