diff mbox series

[kvm-unit-tests,v1,5/7] x86 UEFI: Exit QEMU with return code

Message ID 20211031055634.894263-6-zxwang42@gmail.com (mailing list archive)
State New, archived
Headers show
Series x86_64 UEFI set up process refactor and scripts fixes | expand

Commit Message

Zixuan Wang Oct. 31, 2021, 5:56 a.m. UTC
From: Zixuan Wang <zxwang42@gmail.com>

kvm-unit-tests runner scripts parse QEMU exit code to determine if a
test case runs successfully. But the UEFI 'reset_system' function always
exits QEMU with code 0, even if the test case returns a non-zero code.

This commit fixes this issue by replacing the 'reset_system' call with
an 'exit' call, which ensures QEMU exit with the correct code.

Signed-off-by: Zixuan Wang <zxwang42@gmail.com>
---
 lib/efi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini Oct. 31, 2021, 10:01 a.m. UTC | #1
On 31/10/21 06:56, Zixuan Wang wrote:
> From: Zixuan Wang <zxwang42@gmail.com>
> 
> kvm-unit-tests runner scripts parse QEMU exit code to determine if a
> test case runs successfully. But the UEFI 'reset_system' function always
> exits QEMU with code 0, even if the test case returns a non-zero code.
> 
> This commit fixes this issue by replacing the 'reset_system' call with
> an 'exit' call, which ensures QEMU exit with the correct code.
> 
> Signed-off-by: Zixuan Wang <zxwang42@gmail.com>
> ---
>   lib/efi.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index 99eb00c..cc0386c 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -87,7 +87,7 @@ efi_status_t efi_get_system_config_table(efi_guid_t table_guid, void **table)
>   
>   efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>   {
> -	int ret;
> +	unsigned long ret;

Why this change?

>   	efi_status_t status;
>   	efi_bootinfo_t efi_bootinfo;
>   
> @@ -134,14 +134,14 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>   	ret = main(__argc, __argv, __environ);
>   
>   	/* Shutdown the guest VM */
> -	efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, ret, 0, NULL);
> +	exit(ret);
>   
>   	/* Unreachable */
>   	return EFI_UNSUPPORTED;
>   
>   efi_main_error:
>   	/* Shutdown the guest with error EFI status */
> -	efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, status, 0, NULL);
> +	exit(status);
>   
>   	/* Unreachable */
>   	return EFI_UNSUPPORTED;

It's better to keep the exit() *and* the efi_rs_call(), I think, in case 
the testdev is missing and therefore the exit() does not work.

Paolo
Zixuan Wang Oct. 31, 2021, 9:36 p.m. UTC | #2
On Sun, Oct 31, 2021 at 3:02 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 31/10/21 06:56, Zixuan Wang wrote:
> > From: Zixuan Wang <zxwang42@gmail.com>
> >   efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> >   {
> > -     int ret;
> > +     unsigned long ret;
>
> Why this change?

Didn't notice this, it should be int, thanks for pointing it out!

> >       efi_status_t status;
> >       efi_bootinfo_t efi_bootinfo;
> >
> > @@ -134,14 +134,14 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> >       ret = main(__argc, __argv, __environ);
> >
> >       /* Shutdown the guest VM */
> > -     efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, ret, 0, NULL);
> > +     exit(ret);
> >
> >       /* Unreachable */
> >       return EFI_UNSUPPORTED;
> >
> >   efi_main_error:
> >       /* Shutdown the guest with error EFI status */
> > -     efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, status, 0, NULL);
> > +     exit(status);
> >
> >       /* Unreachable */
> >       return EFI_UNSUPPORTED;
>
> It's better to keep the exit() *and* the efi_rs_call(), I think, in case
> the testdev is missing and therefore the exit() does not work.
>
> Paolo
>

I agree, I think there are three possible solutions:

1. keep both exit() and efi_rs_call() here, or
2. define a new function efi_exit() that calls both exit() and efi_rs_call(), or
3. add efi_rs_call() to the end of exit() function (defined in
lib/x86/io.c), so many other calls to exit() can utilize EFI exit as a
backup

Best regards,
Zixuan
diff mbox series

Patch

diff --git a/lib/efi.c b/lib/efi.c
index 99eb00c..cc0386c 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -87,7 +87,7 @@  efi_status_t efi_get_system_config_table(efi_guid_t table_guid, void **table)
 
 efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 {
-	int ret;
+	unsigned long ret;
 	efi_status_t status;
 	efi_bootinfo_t efi_bootinfo;
 
@@ -134,14 +134,14 @@  efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 	ret = main(__argc, __argv, __environ);
 
 	/* Shutdown the guest VM */
-	efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, ret, 0, NULL);
+	exit(ret);
 
 	/* Unreachable */
 	return EFI_UNSUPPORTED;
 
 efi_main_error:
 	/* Shutdown the guest with error EFI status */
-	efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, status, 0, NULL);
+	exit(status);
 
 	/* Unreachable */
 	return EFI_UNSUPPORTED;