| Message ID | 20200123003603.11610-1-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | tests/boot-serial-test: Allow the HPPA machine to shudown | expand |
On 1/22/20 2:36 PM, 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. Well, SeaBIOS implements it as the magic QEMU halt instruction, # These are artificial instructions used by QEMU firmware. # They are allocated from the unassigned instruction space. halt 1111 1111 1111 1101 1110 1010 1101 0000 reset 1111 1111 1111 1101 1110 1010 1101 0001 followed by an infinite loop, probably to avoid a compiler warning and Just In Case. We really should halt here, unless shutdown is disabled. > - qts = qtest_initf("%s %s -M %s -no-shutdown " > + qts = qtest_initf("%s %s -M %s %s " > "-chardev file,id=serial0,path=%s " > "-serial chardev:serial0 -accel tcg -accel kvm %s", > codeparam, code ? codetmp : "", test->machine, > + shutdown ? "" : "-no-shutdown", > serialtmp, test->extra); And thus avoiding the -no-shutdown should in fact shutdown. Are you saying this doesn't happen, or what? I think I got lost with the rest of the message... r~
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? Sounds like a bug in another place, and not in the boot-serial-test. Thomas
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.
On 1/23/20 4:27 AM, Richard Henderson wrote: > On 1/22/20 2:36 PM, 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. > > Well, SeaBIOS implements it as the magic QEMU halt instruction, > > # These are artificial instructions used by QEMU firmware. > # They are allocated from the unassigned instruction space. > halt 1111 1111 1111 1101 1110 1010 1101 0000 > reset 1111 1111 1111 1101 1110 1010 1101 0001 > > followed by an infinite loop, probably to avoid a compiler warning and Just In > Case. We really should halt here, unless shutdown is disabled. > >> - qts = qtest_initf("%s %s -M %s -no-shutdown " >> + qts = qtest_initf("%s %s -M %s %s " >> "-chardev file,id=serial0,path=%s " >> "-serial chardev:serial0 -accel tcg -accel kvm %s", >> codeparam, code ? codetmp : "", test->machine, >> + shutdown ? "" : "-no-shutdown", >> serialtmp, test->extra); > > And thus avoiding the -no-shutdown should in fact shutdown. > > Are you saying this doesn't happen, or what? I think I got lost with the rest > of the message... "If the command-line option "-no-shutdown" has been specified, qemu will not exit, and a STOP event will eventually follow the SHUTDOWN event" What I see is the HPPA firmware calling HALT, then QEMU doesn't exit. The QTest keeps reading the console for "SeaBIOS wants SYSTEM HALT.". When this string is displayed on the graphic output instead of the serial, the QTest keep reading indefinitely, and the test never succeeds/fails, it is just stuck.
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. 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. So please disregard this patch. Thanks, Phil.
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
On 1/24/20 8:02 AM, Thomas Huth wrote: > 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. Good idea. With QMP it is easy: { "execute": "query-status" } { "return": { "status": "shutdown", "singlestep": false, "running": false } }
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c index 05c7f44457..4e92f292f5 100644 --- a/tests/qtest/boot-serial-test.c +++ b/tests/qtest/boot-serial-test.c @@ -188,6 +188,17 @@ static void test_machine(const void *data) const uint8_t *code = NULL; QTestState *qts; int ser_fd; + bool shutdown; + + /* + * This test uses SeaBIOS on HPPA, and expects to read the + * "SeaBIOS wants SYSTEM HALT" string. A 'SYSTEM HALT' really + * halts the CPU. If SeaBIOS does not use the expected serial + * port but another device, we might poll the console + * indefinitely while the machine is halted. + * Keep using this option for all the other tests. + */ + shutdown = !strcmp(test->expect, "SeaBIOS wants SYSTEM HALT"); ser_fd = mkstemp(serialtmp); g_assert(ser_fd != -1); @@ -215,10 +226,11 @@ static void test_machine(const void *data) * Make sure that this test uses tcg if available: It is used as a * fast-enough smoketest for that. */ - qts = qtest_initf("%s %s -M %s -no-shutdown " + qts = qtest_initf("%s %s -M %s %s " "-chardev file,id=serial0,path=%s " "-serial chardev:serial0 -accel tcg -accel kvm %s", codeparam, code ? codetmp : "", test->machine, + shutdown ? "" : "-no-shutdown", serialtmp, test->extra); if (code) { unlink(codetmp);
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 [*] https://github.com/qemu/seabios-hppa/blob/1630ac7d65/src/parisc/parisc.c#L138 Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- I first used: shutdown = !strcmp(test->arch, "hppa"); Then remembered we'll have other HPPA machines soon. Instead of: shutdown = !strcmp(test->machine, "hppa"); I find the check on the SeaBIOS specific string more accurate (since someone might add some assembly lines as the other tests). --- tests/qtest/boot-serial-test.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)