On Mon, Sep 06, 2021 at 11:20:31AM +0100, Alexandru Elisei wrote:
> Hi Drew,
>
> Sorry for taking so long to reply, been busy with other things.
>
> On 7/12/21 5:36 PM, Andrew Jones wrote:
> > On Fri, Jul 02, 2021 at 05:31:18PM +0100, Alexandru Elisei wrote:
> >> The arm64 tests can be run under kvmtool, which doesn't emulate a
> >> chr-testdev device. In preparation for adding run script support for
> >> kvmtool, print the test exit status so the scripts can pick it up and
> >> correctly mark the test as pass or fail.
> >>
> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> >> ---
> >> lib/chr-testdev.h | 1 +
> >> lib/arm/io.c | 10 +++++++++-
> >> lib/chr-testdev.c | 5 +++++
> >> 3 files changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/chr-testdev.h b/lib/chr-testdev.h
> >> index ffd9a851aa9b..09b4b424670e 100644
> >> --- a/lib/chr-testdev.h
> >> +++ b/lib/chr-testdev.h
> >> @@ -11,4 +11,5 @@
> >> */
> >> extern void chr_testdev_init(void);
> >> extern void chr_testdev_exit(int code);
> >> +extern bool chr_testdev_available(void);
> >> #endif
> >> diff --git a/lib/arm/io.c b/lib/arm/io.c
> >> index 343e10822263..9e62b571a91b 100644
> >> --- a/lib/arm/io.c
> >> +++ b/lib/arm/io.c
> >> @@ -125,7 +125,15 @@ extern void halt(int code);
> >>
> >> void exit(int code)
> >> {
> >> - chr_testdev_exit(code);
> >> + if (chr_testdev_available()) {
> >> + chr_testdev_exit(code);
> > chr_testdev_exit() already has a 'if !vcon goto out' in it, so you can
> > just call it unconditionally. No need for chr_testdev_available().
>
> I'm not sure what you mean. There has to be a way to check if chr-testdev is
> available, and if it's not present on the system, to print the EXIT: STATUS
> message, and vcon is static in chr-testdev.c.
>
> Are you suggesting that we move the message to chr_testdev_exit(code)?
I'm saying you can unconditionally call chr_testdev_exit(), because it
only conditionally does anything, and on the same condition that you're
adding (vcon != NULL).
$ /usr/bin/qemu-system-aarch64 -nodefaults -machine virt,accel=tcg -cpu cortex-a57 -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel arm/selftest.flat
ABORT: selftest: no test specified
SUMMARY: 0 tests
$ echo $?
127
$ /usr/bin/qemu-system-aarch64 -nodefaults -machine virt,accel=tcg -cpu cortex-a57 -display none -serial stdio -kernel arm/selftest.flat
ABORT: selftest: no test specified
SUMMARY: 0 tests
$ echo $?
0
See, no explosions when the device is removed. Just a lack of return code.
Also, since chr_testdev_exit() exits, any calls after it won't happen. So
the exit print statement doesn't need to be in an else clause. That said,
I think the print statement should come first in order to also put it in
the qemu output logs. We might as well have consistent output between qemu
and kvmtool.
Thanks,
drew
>
> Thanks,
>
> Alex
>
> >
> >> + } else {
> >> + /*
> >> + * Print the test return code in the format used by chr-testdev
> >> + * so the runner script can parse it.
> >> + */
> >> + printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
> >> + }
> >> psci_system_off();
> >> halt(code);
> >> __builtin_unreachable();
> >> diff --git a/lib/chr-testdev.c b/lib/chr-testdev.c
> >> index b3c641a833fe..301e73a6c064 100644
> >> --- a/lib/chr-testdev.c
> >> +++ b/lib/chr-testdev.c
> >> @@ -68,3 +68,8 @@ void chr_testdev_init(void)
> >> in_vq = vqs[0];
> >> out_vq = vqs[1];
> >> }
> >> +
> >> +bool chr_testdev_available(void)
> >> +{
> >> + return vcon != NULL;
> >> +}
> >> --
> >> 2.32.0
> >>
> > Thanks,
> > drew
> >
>
