On 1/25/19 4:14 PM, Andre Przywara wrote: > On Fri, 25 Jan 2019 17:05:45 +0100 > Andrew Jones <drjo...@redhat.com> 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.
I don't use the kvm-unit-tests run script, that is correct. I would prefer that I don't change arm/run and keep the function exit() like you originally suggested: void exit(int code) { chr_testdev_exit(code); psci_system_off(); halt(code); __builtin_unreachable(); } If anyone is interested, I or Andre can post patches for the run scripts as a separate set. Is that alright with you, Andrew? _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm