diff mbox series

[SRU,B/C/D/E,bionic/master-next,1/1] LP#1836079 efi: efi_get_memory_map -- increase map headroom

Message ID 20191106170640.83841-1-apw@canonical.com
State New
Headers show
Series [SRU,B/C/D/E,bionic/master-next,1/1] LP#1836079 efi: efi_get_memory_map -- increase map headroom | expand

Commit Message

Andy Whitcroft Nov. 6, 2019, 5:06 p.m. UTC
We are seeing some EFI based machines failing to boot hard in the EFI
stub:

    exit_boot() failed!
    efi_main() failed!

This seems to occur when the bootloader (grub2 in this case) has had to
manipulate some additional files.  We tracked this down to the memory map
dance efi_get_memory_map().  Basically we attempt to close boot services
and it informs us it cannot do so because it failed to record the updated
memory map.  This occurs when there is insufficient space in the passed
memory map buffer to record changes during the operation.  At the point
when this occurs we are unable to reallocate the buffer so we panic.

To avoid this we allocate some additional entries in the buffer to cover
any additional entries.  These are currently insufficient for these
machines in this use case.  Increase EFI_MMAP_NR_SLACK_SLOTS to provide
space for more map modifications.

BugLink: http://bugs.launchpad.net/bugs/1836079
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stefan Bader Nov. 8, 2019, 9:43 a.m. UTC | #1
On 06.11.19 18:06, Andy Whitcroft wrote:
> We are seeing some EFI based machines failing to boot hard in the EFI
> stub:
> 
>     exit_boot() failed!
>     efi_main() failed!
> 
> This seems to occur when the bootloader (grub2 in this case) has had to
> manipulate some additional files.  We tracked this down to the memory map
> dance efi_get_memory_map().  Basically we attempt to close boot services
> and it informs us it cannot do so because it failed to record the updated
> memory map.  This occurs when there is insufficient space in the passed
> memory map buffer to record changes during the operation.  At the point
> when this occurs we are unable to reallocate the buffer so we panic.
> 
> To avoid this we allocate some additional entries in the buffer to cover
> any additional entries.  These are currently insufficient for these
> machines in this use case.  Increase EFI_MMAP_NR_SLACK_SLOTS to provide
> space for more map modifications.
> 
> BugLink: http://bugs.launchpad.net/bugs/1836079

^this is a private bug and we normally use https

> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> ---
> 

also the subject is confusing having multible series (and btw, Cosmic is EOL)
and then again bionic...

 drivers/firmware/efi/libstub/efi-stub-helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 39f87e6dac5c..957dff5a26a2 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -49,7 +49,7 @@ int __pure novamap(void)
>  	return __novamap;
>  }
>  
> -#define EFI_MMAP_NR_SLACK_SLOTS	8
> +#define EFI_MMAP_NR_SLACK_SLOTS	16
>  
>  struct file_info {
>  	efi_file_handle_t *handle;
>
Andy Whitcroft Nov. 8, 2019, 9:59 a.m. UTC | #2
On Fri, Nov 08, 2019 at 10:43:53AM +0100, Stefan Bader wrote:
> On 06.11.19 18:06, Andy Whitcroft wrote:
> > We are seeing some EFI based machines failing to boot hard in the EFI
> > stub:
> > 
> >     exit_boot() failed!
> >     efi_main() failed!
> > 
> > This seems to occur when the bootloader (grub2 in this case) has had to
> > manipulate some additional files.  We tracked this down to the memory map
> > dance efi_get_memory_map().  Basically we attempt to close boot services
> > and it informs us it cannot do so because it failed to record the updated
> > memory map.  This occurs when there is insufficient space in the passed
> > memory map buffer to record changes during the operation.  At the point
> > when this occurs we are unable to reallocate the buffer so we panic.
> > 
> > To avoid this we allocate some additional entries in the buffer to cover
> > any additional entries.  These are currently insufficient for these
> > machines in this use case.  Increase EFI_MMAP_NR_SLACK_SLOTS to provide
> > space for more map modifications.
> > 
> > BugLink: http://bugs.launchpad.net/bugs/1836079
> 
> ^this is a private bug and we normally use https
> 
> > Signed-off-by: Andy Whitcroft <apw@canonical.com>
> > ---
> > 
> 
> also the subject is confusing having multible series (and btw, Cosmic is EOL)
> and then again bionic...
> 
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > index 39f87e6dac5c..957dff5a26a2 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -49,7 +49,7 @@ int __pure novamap(void)
> >  	return __novamap;
> >  }
> >  
> > -#define EFI_MMAP_NR_SLACK_SLOTS	8
> > +#define EFI_MMAP_NR_SLACK_SLOTS	16
> >  
> >  struct file_info {
> >  	efi_file_handle_t *handle;
> > 
> 
> 


Yeah it seems I rather botched this one, and there I was thinking I was
infalable.

:)

-apw
diff mbox series

Patch

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 39f87e6dac5c..957dff5a26a2 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -49,7 +49,7 @@  int __pure novamap(void)
 	return __novamap;
 }
 
-#define EFI_MMAP_NR_SLACK_SLOTS	8
+#define EFI_MMAP_NR_SLACK_SLOTS	16
 
 struct file_info {
 	efi_file_handle_t *handle;