Re: [Qemu-ppc] [Qemu-devel] [PULL 00/66] ppc-for-2.8 queue 20160906

[Thomas Huth]

On 06.09.2016 16:04, Peter Maydell wrote:
> On 6 September 2016 at 04:39, David Gibson <address@hidden> wrote:
>> The following changes since commit e87d397e5ef66276ccc49b829527d605ca07d0ad:
>>
>>   Open 2.8 development tree (2016-09-05 11:38:54 +0100)
>>
>> are available in the git repository at:
>>
>>   git://github.com/dgibson/qemu.git tags/ppc-for-2.8-20160906
>>
>> for you to fetch changes up to 2ccbe3e29adb7c016bc5525d38d2a6e938c481af:
>>
>>   tests: Check serial output of firmware boot of some machines (2016-09-06 
>> 10:28:17 +1000)
>>
>> ----------------------------------------------------------------
>> ppc patch queue for 2016-Sep-6
>>
>> This is my first pull request for the newly opened qemu-2.8 tree.  It
>> contains a heap of things that were too late for 2.7 and have been
>> queued for a while.  In particular:
>>     * A number of preliminary patches for the powernv machine type
>>         * A substantial cleanup of exception handling which will be
>>           necessary to support running a TCG with hypervisor
>>           facilities
>>     * A start on support for POWER9
>>         * Some TCG implementations for new POWER9 instructions
>>         * Some TCG and related cleanups in preparation for POWER9
>>     * Some assorted TCG optimizations
>>     * An implementation of the H_CHANGE_LOGICAL_LAN_MAC hypercall
>>       which allows the MAC address to be changed on the PAPR virtual
>>       NIC.
>>     * Add some extra test cases for several machines (this isn't
>>       strictly in the ppc code, but is most value to ppc)
>>
>> ----------------------------------------------------------------
> 
> Hi. This fails 'make check' on ppc64be:
> 
> QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64
> QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-
> $((RANDOM % 255 + 1))} gtester -k --verbose -m=quick
> tests/spapr-phb-test tests/endianness-test tests/boot-order-test
> tests/prom-env-test tests/drive_del-test tests/postcopy-test
> tests/boot-serial-test tests/device-introspect-test tests/qom-test
> TEST: tests/spapr-phb-test... (pid=14553)
>   /ppc64/spapr-phb/device:                                             OK
> PASS: tests/spapr-phb-test
> TEST: tests/endianness-test... (pid=14558)
>   /ppc64/endianness/mac99:                                             OK
>   /ppc64/endianness/pseries:                                           OK
>   /ppc64/endianness/split/mac99:                                       OK
>   /ppc64/endianness/split/pseries:                                     OK
>   /ppc64/endianness/combine/mac99:                                     OK
>   /ppc64/endianness/combine/pseries:                                   OK
> PASS: tests/endianness-test
> TEST: tests/boot-order-test... (pid=14577)
>   /ppc64/boot-order/prep:                                              OK
>   /ppc64/boot-order/pmac_oldworld:                                     OK
>   /ppc64/boot-order/pmac_newworld:                                     OK
> PASS: tests/boot-order-test
> TEST: tests/prom-env-test... (pid=14611)
>   /ppc64/prom-env/mac99:                                               **
> ERROR:/home/pm215/qemu/tests/prom-env-test.c:41:check_guest_memory:
> assertion failed (signature == MAGIC): (0x00000000 == 0xcafec0de)
> FAIL
> GTester: last random seed: R02Se9508b48e788a26453279aa61c24d15d
> (pid=14758)
>   /ppc64/prom-env/g3beige:                                             **
> ERROR:/home/pm215/qemu/tests/prom-env-test.c:41:check_guest_memory:
> assertion failed (signature == MAGIC): (0x00000000 == 0xcafec0de)
> FAIL

The bad commit is: "ppc: Speed up load/store multiple"

There are two "#if defined(HOST_WORDS_BIGENDIAN)" sections in this patch
which are both bad: The memcpy tries to copy 32-bit values into 64-bit
registers, which of course does not work (unless you compile this code
for a 32-bit host only where the size of a gpr is only 32-bit).
I'd suggest to remove the "#if defined(HOST_WORDS_BIGENDIAN)" part and
use cpu_to_be32() and friends instead of the bswap32() here?

 Thomas