On 4 July 2016 at 10:15, Cédric Le Goater <c...@kaod.org> wrote:
> On 07/02/2016 08:05 PM, mar.krzeminski wrote:
>>
>>
>> W dniu 28.06.2016 o 20:24, Cédric Le Goater pisze:
>>> +static void test_erase_sector(void)
>>> +{
>>> + uint32_t some_page_addr = 0x600 * PAGE_SIZE;
>>> + uint32_t page[PAGE_SIZE / 4];
>>> + int i;
>>> +
>>> + spi_conf(CONF_ENABLE_W0);
>>> +
>>> + spi_ctrl_start_user();
>>> + writeb(AST2400_FLASH_BASE, WREN);
>>> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
>>> + writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
>>> + writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
>> Hi,
>>
>> I think you should not make any byte swapping because
>> qtest for all write* instructions (see qtest.c:377).
>
> yes. on a BE, we should not byte swap.
>
> When on a LE host, the cpu_to_be32() call in the test byte-swaps
> the address, tswap32() in qtest.c does nothing as the host endian
> and guest endian are in sync and so the address reaches the
> controller region in the expected order.
>
> When on a BE host, the cpu_to_be32() does nothing but tswap32() in
> qtest.c reverses the order and so the address reaches the controller
> region in the wrong order (LE).
>
> I see two possible fixes :
>
> 1 - replace the read* routines by out* (no tswap there)
This would be wrong: "out*" is a request for an I/O access,
which is meaningless for ARM. (On x86 it's an actual outb/outw/outl
instruction.) In any case it wouldn't change the number of
byteswaps that happen:
* "outl" qtest command calls cpu_outl(), which calls
stl_p() on the value, to do "store in target endianness
to buffer"; stl_p() will do a byteswap if the target and
host endianness differ. The buffer is then passed to
address_space_write(), which is a wrapper for
address_space_rw(), which treats its buffer as a pile
of bytes to be written.
* "writel" qtest command does a tswap32s() on the data, which
converts it to target endianness in a local variable.
The address of the variable is then passed to
cpu_physical_memory_write(), which is just a thin wrapper
around address_space_rw(), which as above treats its
buffer as a pile of bytes.
So the two things end up the same, it's just that for writel
we do the conversion from a host-ordered value to a
target-ordered value in a direct tswapl(), but for outl
this happens implicitly in cpu_outl()'s stl_p().
> 2 - add an extra byteswap on BE hosts using qtest_big_endian() as
> in virtio-blk-test.c. see virtio_blk_fix_request()
What is the test actually supposed to be doing?
writel() says "write this 32 bit value as if the (guest) CPU
wrote it to memory with a 32-bit write instruction".
Your problem here is entirely on the test code side, I think.
It should be issuing the same commands down the qtest protocol
for any host endianness, but as it stands on a little endian
host it will issue "writel 0x20000000 0x600" whereas on a big
endian host it will issue "writel 0x20000000 0x60000".
(virtio is special because it has some weird ideas about
endianness due to being a virtual-only device with a spec
written by all-the-world-is-little-endian x86 people, so
it's a bad example to copy.)
thanks
-- PMM
