On Fri, Jan 25, 2019 at 04:31:37PM +0100, Andrew Jones wrote: > On Fri, Jan 25, 2019 at 02:56:30PM +0000, Alexandru Elisei wrote: > > > > On 1/24/19 1:35 PM, Andrew Jones wrote: > > > On Thu, Jan 24, 2019 at 02:00:20PM +0100, Andrew Jones wrote: > > >> [..] > > >> chr_testdev_init() ensures vcon is NULL if it fails to initialize. > > >> chr_testdev_exit() immediately returns if vcon is NULL. This was > > >> done by design to allow fallback exits to be placed below the > > >> chr_testdev_exit call, e.g. halt(). > > >> > > >> We should be able to drop patch 3/7 and change exit() to this > > >> > > >> void exit(int code) > > >> { > > >> chr_testdev_exit(code); > > >> psci_system_off(); > > >> halt(code); > > >> __builtin_unreachable(); > > >> } > > >> > > > There's also a framework for exits that can't return status codes. powerpc > > > uses it. Before exiting with psci_system_off we need to make this print > > > statement > > > > > > printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1); > > > > > > And run_qemu in arm/run needs to be changed to run_qemu_status. It's > > > hacky, but maybe we can live with it until kvmtool offers some sort of > > > debug exit. > > > > > > Thanks, > > > drew > > > > I can make the change, but if I understand the scripts/arch-run.bash code > > correctly, run_qemu() will check if QEMU was terminated because of a signal and > > adjust the return code to take that into account. Using run_qemu_status() means > > that check won't be made when the tests are run under QEMU, is that acceptable? > > > > run_qemu_status() first calls run_qemu(). If the return value from > run_qemu() is 1, then it'll change the value to whatever the EXIT: > status line says it should be. If run_qemu() detected that a signal > caused the exit, then the return value won't be 1. In that case > run_qemu_status() will simply propagate the value to the caller. > > It might be worth doing a few tests to ensure that all works out as > designed, but I'm pretty sure it should. > It just occurred to me that you must not be using the run scripts anyway, since they would require further patches to work. In that case, there's no need to change arm/run unless you also provide those additional patches. BTW, I wouldn't be opposed to a second run script, rather than trying to make one script work for both qemu and kvmtool. Ideally both scripts would be driven by the same higher level scripts using the same unittests.cfg file though. The unittests.cfg extra_params field will make that a bit challenging, but otherwise I think adding a few new helper functions to scripts/arch-run.bash may be all that's necessary. drew