Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Thomas Huth]

On 29/04/2019 07.09, Li Qiang wrote:
> 
> 
> Li Qiang <address@hidden <mailto:address@hidden>> 于2019年4月25日周
> 四 下午10:29写道：
> 
> 
> 
>     Thomas Huth <address@hidden <mailto:address@hidden>> 于2019年4月
>     25日周四 下午5:57写道：
> 
>         On 24/04/2019 16.06, Li Qiang wrote:
>         > In the disscuss of adding reboot timeout test case:
>         >
>         https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
>         >
>         > Philippe suggested we should uses the only related option for one
>         > specific test. However currently we uses one QTestState for
>         all the
>         > test cases. In order to achieve Philippe's idea, I split the
>         test case
>         > for its own QTestState. As this patchset has changed a lot, I
>         don't bump
>         > the version.
>         >
>         > Change since v1:
>         > Add a patch to store the reboot_timeout as little endian
>         > Fix the endian issue per Thomas's review
> 
>         The test still aborts on a big endian host:
> 
>         $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
>         tests/fw_cfg-test
>         /x86_64/fw_cfg/signature: OK
>         /x86_64/fw_cfg/id: OK
>         /x86_64/fw_cfg/uuid: OK
>         /x86_64/fw_cfg/ram_size: OK
>         /x86_64/fw_cfg/nographic: OK
>         /x86_64/fw_cfg/nb_cpus: OK
>         /x86_64/fw_cfg/max_cpus: OK
>         /x86_64/fw_cfg/numa: OK
>         /x86_64/fw_cfg/boot_menu: OK
>         /x86_64/fw_cfg/reboot_timeout: **
>         
> ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
>         assertion failed (reboot_timeout == 15): (251658240 == 15)
>         Aborted
> 
>         251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e.
>         you still
>         got an endianess issue somewhere in the code.
> 
> 
> 
>     Hmmmm,
> 
>     I have thought a long time, still can't point where is wrong.
> 
>     Let's from the result: 
>     0x0f000000 in the big endian laid as this:
>     low ---> high
>     0x0f 00 00 00
> 
>     As I have swapped before the compare so it is read as this:
>     low ---> high
>     00 00 00 0x0f
> 
>     However from the store side:
>     the 15 in big endian is:
>     low ---> high
>     00 00 00 0x0f
> 
>     But Before I store it, I convert it to little endian, so following
>     should be stored:
>     low ---> high
>     0x0f 00 00 00
> 
>     Do you apply the patch 3 and recompile the qemu binary?
> 
> 
> 
> Hello Thomas, 
> I have tested again this and just store it as big endian(so that the
> store/load has different endianness), 
> I don't see any error. 

Uh, now this is embarrassing... I just tried again to see whether I
could find the issue, but now the test passes without problems!
I guess I simply only did a "make tests/fw_cfg-test" before and forgot
to recompile qemu itself. Big sorry for this!

Anyway, patch series works fine for me, so for the series:

Tested-by: Thomas Huth <address@hidden>

> Also, can we add these test sceneries(big-endian host) in our CI? so
> that the bot can report for every commit.

Patchew only runs on x86, but Peter has some big endian hosts for his
acceptance tests - so problems should be found during PULL requests at
least.

 Thomas