Re: [PATCH] tests/boot-serial-test: Allow the HPPA machine to shudown

[Thomas Huth <thuth@redhat.com>]

On 23/01/2020 22.37, Philippe Mathieu-Daudé wrote:
> On 1/23/20 7:29 PM, Philippe Mathieu-Daudé wrote:
>> On 1/23/20 5:39 AM, Thomas Huth wrote:
>>> On 23/01/2020 01.36, Philippe Mathieu-Daudé wrote:
>>>> The boot-serial test uses SeaBIOS on HPPA, and expects to read the
>>>> "SeaBIOS wants SYSTEM HALT" string, see [*]:
>>>>
>>>>   122 void __VISIBLE __noreturn hlt(void)
>>>>   123 {
>>>>   124     if (pdc_debug)
>>>>   125         printf("HALT initiated from %p\n", 
>>>> __builtin_return_address(0));
>>>>   126     printf("SeaBIOS wants SYSTEM HALT.\n\n");
>>>>   127     asm volatile("\t.word 0xfffdead0": : :"memory");
>>>>   128     while (1);
>>>>   129 }
>>>>
>>>> A 'SYSTEM HALT' would really halts the CPU, but SeaBIOS implements
>>>> it as an infinite loop.
>>>>
>>>> If SeaBIOS does not use the expected serial port but another device,
>>>> we might poll the console indefinitely while the machine is halted.
>>>>
>>>> Allow the HPPA machine to 'shutdown'. When it does, we'll get
>>>> a qtest error:
>>>>
>>>>    $ make check-qtest-hppa
>>>>      TEST    check-qtest-hppa: tests/qtest/boot-serial-test
>>>>    ** (tests/qtest/boot-serial-test:31924): ERROR **: 01:12:37.604:
>>>> Failed to find expected string. Please check
>>>> '/tmp/qtest-boot-serial-sjxoM6Q'
>>>>    ERROR - Bail out! FATAL-ERROR: Failed to find expected string.
>>>> Please check '/tmp/qtest-boot-serial-sjxoM6Q'
>>>>    make: *** [tests/Makefile.include:628: check-qtest-hppa] Error 1
>>>
>>> The tests are run with -no-shutdown. Why does qemu exit in that case?
>>
>> Because the HPPA firmware called HALT.
>>
>>> Sounds like a bug in another place, and not in the boot-serial-test.
>>
>> Yes, the bug is elsewhere, but with the bug the boot-serial-test hangs
>> forever.... No output on the console, qtest waiting indefinitely.
> 
> Richard explained me on IRC what you probably meant, which was not
> obvious to me at first.

I think I also did not really understand what you tried to do here ;-)

The -no-shutdown is required, too, otherwise you could get a race
between the test reading the serial output and the termination of QEMU,
see commit 7150d34a1d60851d73d6ab6783b12b1d25e68f86, and I think we need
that for HPPA, too.

> Now I found in check_guest_output():
> 
>     /* Poll serial output... */
>     while (1) {
>         ...
>         /* Wait at most 360 seconds.  */
>         now = time(NULL);
>         if (now - start >= 360) {
>             break;
>         }
>         g_usleep(10000);
>     }
> 
> $ QTEST_QEMU_BINARY=hppa-softmmu/qemu-system-hppa \
>   time tests/qtest/boot-serial-test -k
> /hppa/boot-serial/hppa:
> ** (tests/qtest/boot-serial-test:18604): ERROR **: 22:33:25.010: Failed
> to find expected string. Please check '/tmp/qtest-boot-serial-sZq7ljM'
> Command terminated by signal 5
> 0.31user 0.66system 6:00.07elapsed 0%CPU
> 
> Indeed the test fails after 6min, I guess I didn't expect that much
> while testing interactively.

Yeah, the huge timeout is ugly, but it is required for very slow hosts,
see commit 627fce617868df87b3757375a2a0318ad2beb381.

So if you want to "fix" your problem, I think you could maybe add a
check for the QEMU events here instead. Or add a check to see whether
the registers of the guest change between iterations to make sure that
the guest is still alive (that way you also handle the case that the
guest crashed and loops forever in a "branch self" instruction). Or
simply continue with the 360s timeout, it's long, but it should trigger
only in case of other bugs, so it's maybe not worth the effort to add
more logic here.

 Thomas