On Fri, 25 Jan 2019 17:05:45 +0100 Andrew Jones <drjones@xxxxxxxxxx> wrote: Hi, > 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. Yeah, I had some patches along those lines: split test parameters from QEMU parameters, abstract common stuff like number of cores and amount of memory, mark tests as QEMU only and so on. And I had a separate run script for kvmtool, IIRC. If there is interest I can try to post them, but I would consider this an additional effort on top of this series. Cheers, Andre.