| Message ID | 20190124111634.4727-6-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm/arm64: Add support for running under kvmtool | expand |
On Thu, Jan 24, 2019 at 11:16:32AM +0000, Alexandru Elisei wrote: > On arm and arm64, kvm-unit-tests uses the QEMU chr-testdev device to shut > down the virtual machine at the end of a test. The function > psci_system_off() provides another mechanism for terminating the virtual > machine. If the chr-testdev device hasn't been initialized successfully, > then use psci_system_off() to terminate the test instead of > chr_testdev_exit(). > > chr-testdev is implemented on top of virtio console. This patch makes it > possible for a virtual machine manager which doesn't have support for > chr-testdev, but has been configured not to emulate a virtio console, to > gracefully terminate a virtual machine after a test has been completed. > > There is one limitation to using psci_system_off() to terminate a test: > chr-testdev allows kvm-unit-tests to specify an exit code; > psci_system_off() has no such mechanism. > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > lib/arm/io.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/lib/arm/io.c b/lib/arm/io.c > index 87435150f73e..9fe9bd0bf659 100644 > --- a/lib/arm/io.c > +++ b/lib/arm/io.c > @@ -11,6 +11,7 @@ > #include <libcflat.h> > #include <devicetree.h> > #include <chr-testdev.h> > +#include "arm/asm/psci.h" > #include <asm/spinlock.h> > #include <asm/io.h> > > @@ -18,6 +19,8 @@ > > extern void halt(int code); > > +static bool testdev_enabled; > + > /* > * Use this guess for the pl011 base in order to make an attempt at > * having earlier printf support. We'll overwrite it with the real > @@ -65,8 +68,12 @@ static void uart0_init(void) > > void io_init(void) > { > + int err; > + > uart0_init(); > - chr_testdev_init(); > + err = chr_testdev_init(); > + if (!err) > + testdev_enabled = true; > } > > void puts(const char *s) > @@ -79,7 +86,10 @@ void puts(const char *s) > > void exit(int code) > { > - chr_testdev_exit(code); > + if (testdev_enabled) > + chr_testdev_exit(code); > + else > + psci_system_off(); > halt(code); > __builtin_unreachable(); > } > -- > 2.17.0 > 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(); } Thanks, drew
On Thu, Jan 24, 2019 at 02:00:20PM +0100, Andrew Jones wrote: > On Thu, Jan 24, 2019 at 11:16:32AM +0000, Alexandru Elisei wrote: > > On arm and arm64, kvm-unit-tests uses the QEMU chr-testdev device to shut > > down the virtual machine at the end of a test. The function > > psci_system_off() provides another mechanism for terminating the virtual > > machine. If the chr-testdev device hasn't been initialized successfully, > > then use psci_system_off() to terminate the test instead of > > chr_testdev_exit(). > > > > chr-testdev is implemented on top of virtio console. This patch makes it > > possible for a virtual machine manager which doesn't have support for > > chr-testdev, but has been configured not to emulate a virtio console, to > > gracefully terminate a virtual machine after a test has been completed. > > > > There is one limitation to using psci_system_off() to terminate a test: > > chr-testdev allows kvm-unit-tests to specify an exit code; > > psci_system_off() has no such mechanism. > > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > --- > > lib/arm/io.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/lib/arm/io.c b/lib/arm/io.c > > index 87435150f73e..9fe9bd0bf659 100644 > > --- a/lib/arm/io.c > > +++ b/lib/arm/io.c > > @@ -11,6 +11,7 @@ > > #include <libcflat.h> > > #include <devicetree.h> > > #include <chr-testdev.h> > > +#include "arm/asm/psci.h" > > #include <asm/spinlock.h> > > #include <asm/io.h> > > > > @@ -18,6 +19,8 @@ > > > > extern void halt(int code); > > > > +static bool testdev_enabled; > > + > > /* > > * Use this guess for the pl011 base in order to make an attempt at > > * having earlier printf support. We'll overwrite it with the real > > @@ -65,8 +68,12 @@ static void uart0_init(void) > > > > void io_init(void) > > { > > + int err; > > + > > uart0_init(); > > - chr_testdev_init(); > > + err = chr_testdev_init(); > > + if (!err) > > + testdev_enabled = true; > > } > > > > void puts(const char *s) > > @@ -79,7 +86,10 @@ void puts(const char *s) > > > > void exit(int code) > > { > > - chr_testdev_exit(code); > > + if (testdev_enabled) > > + chr_testdev_exit(code); > > + else > > + psci_system_off(); > > halt(code); > > __builtin_unreachable(); > > } > > -- > > 2.17.0 > > > > 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
On 1/24/19 1:00 PM, Andrew Jones wrote: > On Thu, Jan 24, 2019 at 11:16:32AM +0000, Alexandru Elisei wrote: >> On arm and arm64, kvm-unit-tests uses the QEMU chr-testdev device to shut >> down the virtual machine at the end of a test. The function >> psci_system_off() provides another mechanism for terminating the virtual >> machine. If the chr-testdev device hasn't been initialized successfully, >> then use psci_system_off() to terminate the test instead of >> chr_testdev_exit(). >> >> chr-testdev is implemented on top of virtio console. This patch makes it >> possible for a virtual machine manager which doesn't have support for >> chr-testdev, but has been configured not to emulate a virtio console, to >> gracefully terminate a virtual machine after a test has been completed. >> >> There is one limitation to using psci_system_off() to terminate a test: >> chr-testdev allows kvm-unit-tests to specify an exit code; >> psci_system_off() has no such mechanism. >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >> --- >> lib/arm/io.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/lib/arm/io.c b/lib/arm/io.c >> index 87435150f73e..9fe9bd0bf659 100644 >> --- a/lib/arm/io.c >> +++ b/lib/arm/io.c >> @@ -11,6 +11,7 @@ >> #include <libcflat.h> >> #include <devicetree.h> >> #include <chr-testdev.h> >> +#include "arm/asm/psci.h" >> #include <asm/spinlock.h> >> #include <asm/io.h> >> >> @@ -18,6 +19,8 @@ >> >> extern void halt(int code); >> >> +static bool testdev_enabled; >> + >> /* >> * Use this guess for the pl011 base in order to make an attempt at >> * having earlier printf support. We'll overwrite it with the real >> @@ -65,8 +68,12 @@ static void uart0_init(void) >> >> void io_init(void) >> { >> + int err; >> + >> uart0_init(); >> - chr_testdev_init(); >> + err = chr_testdev_init(); >> + if (!err) >> + testdev_enabled = true; >> } >> >> void puts(const char *s) >> @@ -79,7 +86,10 @@ void puts(const char *s) >> >> void exit(int code) >> { >> - chr_testdev_exit(code); >> + if (testdev_enabled) >> + chr_testdev_exit(code); >> + else >> + psci_system_off(); >> halt(code); >> __builtin_unreachable(); >> } >> -- >> 2.17.0 >> > 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(); > } > > Thanks, > drew I wasn't aware of this design decision, thank you for explaining it. I'll implement the changes you suggested, including dropping patch 3/7: "lib: chr-testdev: Make chr_testdev_init() return status".
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?
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. Thanks, drew
On 1/25/19 3:31 PM, 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. > > Thanks, > drew Now it makes more sense, thank you. I'll make the change and run some QEMU and kvmtool tests.
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
On Fri, 25 Jan 2019 17:05:45 +0100 Andrew Jones <drjones@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.
On 1/25/19 4:14 PM, Andre Przywara wrote: > On Fri, 25 Jan 2019 17:05:45 +0100 > Andrew Jones <drjones@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?
On Fri, Jan 25, 2019 at 04:44:39PM +0000, Alexandru Elisei wrote: > On 1/25/19 4:14 PM, Andre Przywara wrote: > > On Fri, 25 Jan 2019 17:05:45 +0100 > > Andrew Jones <drjones@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? > Yup, that's fine.
diff --git a/lib/arm/io.c b/lib/arm/io.c index 87435150f73e..9fe9bd0bf659 100644 --- a/lib/arm/io.c +++ b/lib/arm/io.c @@ -11,6 +11,7 @@ #include <libcflat.h> #include <devicetree.h> #include <chr-testdev.h> +#include "arm/asm/psci.h" #include <asm/spinlock.h> #include <asm/io.h> @@ -18,6 +19,8 @@ extern void halt(int code); +static bool testdev_enabled; + /* * Use this guess for the pl011 base in order to make an attempt at * having earlier printf support. We'll overwrite it with the real @@ -65,8 +68,12 @@ static void uart0_init(void) void io_init(void) { + int err; + uart0_init(); - chr_testdev_init(); + err = chr_testdev_init(); + if (!err) + testdev_enabled = true; } void puts(const char *s) @@ -79,7 +86,10 @@ void puts(const char *s) void exit(int code) { - chr_testdev_exit(code); + if (testdev_enabled) + chr_testdev_exit(code); + else + psci_system_off(); halt(code); __builtin_unreachable(); }
On arm and arm64, kvm-unit-tests uses the QEMU chr-testdev device to shut down the virtual machine at the end of a test. The function psci_system_off() provides another mechanism for terminating the virtual machine. If the chr-testdev device hasn't been initialized successfully, then use psci_system_off() to terminate the test instead of chr_testdev_exit(). chr-testdev is implemented on top of virtio console. This patch makes it possible for a virtual machine manager which doesn't have support for chr-testdev, but has been configured not to emulate a virtio console, to gracefully terminate a virtual machine after a test has been completed. There is one limitation to using psci_system_off() to terminate a test: chr-testdev allows kvm-unit-tests to specify an exit code; psci_system_off() has no such mechanism. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- lib/arm/io.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)