Re: [PATCH 6/7] WIP: efi: Allow helloworld to exit boot services
Hi Ilias, On Wed, 22 Nov 2023 at 00:39, Ilias Apalodimas wrote: > > Hi Simon, > > On Wed, 22 Nov 2023 at 00:10, Simon Glass wrote: > > > > Hi Heinrich, > > > > On Tue, 21 Nov 2023 at 10:17, Heinrich Schuchardt > > wrote: > > > > > > On 11/21/23 12:35, Simon Glass wrote: > > > > This allows testing of the exit_boot_services call, providing more > > > > coverage of the EFI bootmeth. > > > > > > > > Signed-off-by: Simon Glass > > > > > > I would prefer to keep helloworld.efi benign. > > > > > > Please, create a new EFI binary for testing ExitBootServices(). > > > > Eek...isn't helloworld already used in tets?\ > > Yes, but calling exitboot services from it means we need to be really > careful when/how we call it, since once that app runs the boottime > services along with any attached disks will disappear. > Instead having a single EFI app that calls that and then tests for any > runtime services is more scalable OK I can add a new app. I am still stuck on the actual bug mentioned in the cover letter, though. Regards, Simon
Re: [PATCH 6/7] WIP: efi: Allow helloworld to exit boot services
Hi Simon,
On Wed, 22 Nov 2023 at 00:10, Simon Glass wrote:
>
> Hi Heinrich,
>
> On Tue, 21 Nov 2023 at 10:17, Heinrich Schuchardt wrote:
> >
> > On 11/21/23 12:35, Simon Glass wrote:
> > > This allows testing of the exit_boot_services call, providing more
> > > coverage of the EFI bootmeth.
> > >
> > > Signed-off-by: Simon Glass
> >
> > I would prefer to keep helloworld.efi benign.
> >
> > Please, create a new EFI binary for testing ExitBootServices().
>
> Eek...isn't helloworld already used in tets?\
Yes, but calling exitboot services from it means we need to be really
careful when/how we call it, since once that app runs the boottime
services along with any attached disks will disappear.
Instead having a single EFI app that calls that and then tests for any
runtime services is more scalable
Thanks
/Ilias
>
> >
> > > ---
> > >
> > > lib/efi_loader/helloworld.c | 36 +++-
> > > 1 file changed, 35 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
> > > index 1135d3a3c37e..1fb5fb5a62f2 100644
> > > --- a/lib/efi_loader/helloworld.c
> > > +++ b/lib/efi_loader/helloworld.c
> > > @@ -206,6 +206,26 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
> > > (con_out, u"Cannot open loaded image protocol\r\n");
> > > goto out;
> > > }
> > > +
> > > + {
> > > + ulong ptr = (ulong)loaded_image;
> > > + u16 str[80];
> > > + int i;
> > > +
> > > + for (i = 0; i < 8; i++) {
> > > + uint digit = (ptr >> ((7 - i) * 4)) & 0xf;
> > > +
> > > + if (digit > 9)
> > > + digit = 'a' + digit - 10;
> > > + else
> > > + digit += '0';
> > > + str[i] = digit;
> > > + }
> > > + str[i] = 0;
> > > + con_out->output_string(con_out, str);
> > > + con_out->output_string(con_out, u"\n");
> > > + }
> > > +
> > > print_load_options(loaded_image);
> > >
> > > /* Get the device path to text protocol */
> > > @@ -243,7 +263,21 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
> > > goto out;
> > >
> > > out:
> > > - boottime->exit(handle, ret, 0, NULL);
> > > + /*
> > > + * TODO: Use vendor string to decide whether to call
> > > exit-boot-services
> > > + */
> > > + efi_uintn_t map_size = 0;
> > > + efi_uintn_t map_key;
> > > + efi_uintn_t desc_size;
> > > + u32 desc_version;
> > > +
> > > + ret = boottime->get_memory_map(&map_size, NULL, &map_key,
> > > &desc_size,
> > > +&desc_version);
> > > + con_out->output_string(con_out, u"Exiting boot sevices\n");
> > > +
> > > + boottime->exit_boot_services(handle, map_key);
> > > +
> > > + ret = boottime->exit(handle, ret, 0, NULL);
> >
> > After ExitBootServices() you must not call boot services.
> >
> > Call runtime->reset_system(EFI_RESET_COLD, EFI_SUCCESS, 0, NULL) instead.
>
> OK, will do. That will be ignored by sandbox, right?
>
> For now this is just a hack to try to resolve the problem mentioned.
>
> Regards,
> Simon
Re: [PATCH 6/7] WIP: efi: Allow helloworld to exit boot services
Hi Heinrich,
On Tue, 21 Nov 2023 at 10:17, Heinrich Schuchardt wrote:
>
> On 11/21/23 12:35, Simon Glass wrote:
> > This allows testing of the exit_boot_services call, providing more
> > coverage of the EFI bootmeth.
> >
> > Signed-off-by: Simon Glass
>
> I would prefer to keep helloworld.efi benign.
>
> Please, create a new EFI binary for testing ExitBootServices().
Eek...isn't helloworld already used in tets?
>
> > ---
> >
> > lib/efi_loader/helloworld.c | 36 +++-
> > 1 file changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
> > index 1135d3a3c37e..1fb5fb5a62f2 100644
> > --- a/lib/efi_loader/helloworld.c
> > +++ b/lib/efi_loader/helloworld.c
> > @@ -206,6 +206,26 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
> > (con_out, u"Cannot open loaded image protocol\r\n");
> > goto out;
> > }
> > +
> > + {
> > + ulong ptr = (ulong)loaded_image;
> > + u16 str[80];
> > + int i;
> > +
> > + for (i = 0; i < 8; i++) {
> > + uint digit = (ptr >> ((7 - i) * 4)) & 0xf;
> > +
> > + if (digit > 9)
> > + digit = 'a' + digit - 10;
> > + else
> > + digit += '0';
> > + str[i] = digit;
> > + }
> > + str[i] = 0;
> > + con_out->output_string(con_out, str);
> > + con_out->output_string(con_out, u"\n");
> > + }
> > +
> > print_load_options(loaded_image);
> >
> > /* Get the device path to text protocol */
> > @@ -243,7 +263,21 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
> > goto out;
> >
> > out:
> > - boottime->exit(handle, ret, 0, NULL);
> > + /*
> > + * TODO: Use vendor string to decide whether to call
> > exit-boot-services
> > + */
> > + efi_uintn_t map_size = 0;
> > + efi_uintn_t map_key;
> > + efi_uintn_t desc_size;
> > + u32 desc_version;
> > +
> > + ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
> > +&desc_version);
> > + con_out->output_string(con_out, u"Exiting boot sevices\n");
> > +
> > + boottime->exit_boot_services(handle, map_key);
> > +
> > + ret = boottime->exit(handle, ret, 0, NULL);
>
> After ExitBootServices() you must not call boot services.
>
> Call runtime->reset_system(EFI_RESET_COLD, EFI_SUCCESS, 0, NULL) instead.
OK, will do. That will be ignored by sandbox, right?
For now this is just a hack to try to resolve the problem mentioned.
Regards,
Simon
Re: [PATCH 6/7] WIP: efi: Allow helloworld to exit boot services
On 11/21/23 12:35, Simon Glass wrote:
This allows testing of the exit_boot_services call, providing more
coverage of the EFI bootmeth.
Signed-off-by: Simon Glass
I would prefer to keep helloworld.efi benign.
Please, create a new EFI binary for testing ExitBootServices().
---
lib/efi_loader/helloworld.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
index 1135d3a3c37e..1fb5fb5a62f2 100644
--- a/lib/efi_loader/helloworld.c
+++ b/lib/efi_loader/helloworld.c
@@ -206,6 +206,26 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
(con_out, u"Cannot open loaded image protocol\r\n");
goto out;
}
+
+ {
+ ulong ptr = (ulong)loaded_image;
+ u16 str[80];
+ int i;
+
+ for (i = 0; i < 8; i++) {
+ uint digit = (ptr >> ((7 - i) * 4)) & 0xf;
+
+ if (digit > 9)
+ digit = 'a' + digit - 10;
+ else
+ digit += '0';
+ str[i] = digit;
+ }
+ str[i] = 0;
+ con_out->output_string(con_out, str);
+ con_out->output_string(con_out, u"\n");
+ }
+
print_load_options(loaded_image);
/* Get the device path to text protocol */
@@ -243,7 +263,21 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
goto out;
out:
- boottime->exit(handle, ret, 0, NULL);
+ /*
+* TODO: Use vendor string to decide whether to call exit-boot-services
+*/
+ efi_uintn_t map_size = 0;
+ efi_uintn_t map_key;
+ efi_uintn_t desc_size;
+ u32 desc_version;
+
+ ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
+ &desc_version);
+ con_out->output_string(con_out, u"Exiting boot sevices\n");
+
+ boottime->exit_boot_services(handle, map_key);
+
+ ret = boottime->exit(handle, ret, 0, NULL);
After ExitBootServices() you must not call boot services.
Call runtime->reset_system(EFI_RESET_COLD, EFI_SUCCESS, 0, NULL) instead.
Best regards
Heinrich
/* We should never arrive here */
return ret;
[PATCH 6/7] WIP: efi: Allow helloworld to exit boot services
This allows testing of the exit_boot_services call, providing more
coverage of the EFI bootmeth.
Signed-off-by: Simon Glass
---
lib/efi_loader/helloworld.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
index 1135d3a3c37e..1fb5fb5a62f2 100644
--- a/lib/efi_loader/helloworld.c
+++ b/lib/efi_loader/helloworld.c
@@ -206,6 +206,26 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
(con_out, u"Cannot open loaded image protocol\r\n");
goto out;
}
+
+ {
+ ulong ptr = (ulong)loaded_image;
+ u16 str[80];
+ int i;
+
+ for (i = 0; i < 8; i++) {
+ uint digit = (ptr >> ((7 - i) * 4)) & 0xf;
+
+ if (digit > 9)
+ digit = 'a' + digit - 10;
+ else
+ digit += '0';
+ str[i] = digit;
+ }
+ str[i] = 0;
+ con_out->output_string(con_out, str);
+ con_out->output_string(con_out, u"\n");
+ }
+
print_load_options(loaded_image);
/* Get the device path to text protocol */
@@ -243,7 +263,21 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
goto out;
out:
- boottime->exit(handle, ret, 0, NULL);
+ /*
+* TODO: Use vendor string to decide whether to call exit-boot-services
+*/
+ efi_uintn_t map_size = 0;
+ efi_uintn_t map_key;
+ efi_uintn_t desc_size;
+ u32 desc_version;
+
+ ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
+ &desc_version);
+ con_out->output_string(con_out, u"Exiting boot sevices\n");
+
+ boottime->exit_boot_services(handle, map_key);
+
+ ret = boottime->exit(handle, ret, 0, NULL);
/* We should never arrive here */
return ret;
--
2.43.0.rc1.413.gea7ed67945-goog
