Author: bcran
Date: Fri Sep 27 05:12:28 2019
New Revision: 352788
URL: https://svnweb.freebsd.org/changeset/base/352788

Log:
  MFC r344839:
  
  Add retry loop around GetMemoryMap call to fix fragmentation bug
  
  The call to BS->AllocatePages can cause the memory map to become framented,
  causing BS->GetMemoryMap to return EFI_BUFFER_TOO_SMALL more than once. For
  example this can happen on the MinnowBoard Turbot, causing the boot to stop
  with an error. Avoid this by calling GetMemoryMap in a loop.

Modified:
  stable/12/stand/efi/loader/bootinfo.c
  stable/12/stand/efi/loader/copy.c

Modified: stable/12/stand/efi/loader/bootinfo.c
==============================================================================
--- stable/12/stand/efi/loader/bootinfo.c       Fri Sep 27 02:09:20 2019        
(r352787)
+++ stable/12/stand/efi/loader/bootinfo.c       Fri Sep 27 05:12:28 2019        
(r352788)
@@ -287,12 +287,12 @@ static int
 bi_load_efi_data(struct preloaded_file *kfp)
 {
        EFI_MEMORY_DESCRIPTOR *mm;
-       EFI_PHYSICAL_ADDRESS addr;
+       EFI_PHYSICAL_ADDRESS addr = 0;
        EFI_STATUS status;
        const char *efi_novmap;
        size_t efisz;
        UINTN efi_mapkey;
-       UINTN mmsz, pages, retry, sz;
+       UINTN dsz, pages, retry, sz;
        UINT32 mmver;
        struct efi_map_header *efihdr;
        bool do_vmap;
@@ -323,76 +323,94 @@ bi_load_efi_data(struct preloaded_file *kfp)
        efisz = (sizeof(struct efi_map_header) + 0xf) & ~0xf;
 
        /*
-        * Assgin size of EFI_MEMORY_DESCRIPTOR to keep compatible with
+        * Assign size of EFI_MEMORY_DESCRIPTOR to keep compatible with
         * u-boot which doesn't fill this value when buffer for memory
         * descriptors is too small (eg. 0 to obtain memory map size)
         */
-       mmsz = sizeof(EFI_MEMORY_DESCRIPTOR);
+       dsz = sizeof(EFI_MEMORY_DESCRIPTOR);
 
        /*
-        * It is possible that the first call to ExitBootServices may change
-        * the map key. Fetch a new map key and retry ExitBootServices in that
-        * case.
+        * Allocate enough pages to hold the bootinfo block and the
+        * memory map EFI will return to us. The memory map has an
+        * unknown size, so we have to determine that first. Note that
+        * the AllocatePages call can itself modify the memory map, so
+        * we have to take that into account as well. The changes to
+        * the memory map are caused by splitting a range of free
+        * memory into two, so that one is marked as being loader
+        * data.
         */
+
+       sz = 0;
+
+       /*
+        * Matthew Garrett has observed at least one system changing the
+        * memory map when calling ExitBootServices, causing it to return an
+        * error, probably because callbacks are allocating memory.
+        * So we need to retry calling it at least once.
+        */
        for (retry = 2; retry > 0; retry--) {
-               /*
-                * Allocate enough pages to hold the bootinfo block and the
-                * memory map EFI will return to us. The memory map has an
-                * unknown size, so we have to determine that first. Note that
-                * the AllocatePages call can itself modify the memory map, so
-                * we have to take that into account as well. The changes to
-                * the memory map are caused by splitting a range of free
-                * memory into two (AFAICT), so that one is marked as being
-                * loader data.
-                */
-               sz = 0;
-               BS->GetMemoryMap(&sz, NULL, &efi_mapkey, &mmsz, &mmver);
-               sz += mmsz;
-               sz = (sz + 0xf) & ~0xf;
-               pages = EFI_SIZE_TO_PAGES(sz + efisz);
-               status = BS->AllocatePages(AllocateAnyPages, EfiLoaderData,
-                    pages, &addr);
-               if (EFI_ERROR(status)) {
-                       printf("%s: AllocatePages error %lu\n", __func__,
-                           EFI_ERROR_CODE(status));
-                       return (ENOMEM);
-               }
+               for (;;) {
+                       status = BS->GetMemoryMap(&sz, mm, &efi_mapkey, &dsz, 
&mmver);
+                       if (!EFI_ERROR(status))
+                               break;
 
-               /*
-                * Read the memory map and stash it after bootinfo. Align the
-                * memory map on a 16-byte boundary (the bootinfo block is page
-                * aligned).
-                */
-               efihdr = (struct efi_map_header *)(uintptr_t)addr;
-               mm = (void *)((uint8_t *)efihdr + efisz);
-               sz = (EFI_PAGE_SIZE * pages) - efisz;
+                       if (status != EFI_BUFFER_TOO_SMALL) {
+                               printf("%s: GetMemoryMap error %lu\n", __func__,
+                                  EFI_ERROR_CODE(status));
+                               return (EINVAL);
+                       }
 
-               status = BS->GetMemoryMap(&sz, mm, &efi_mapkey, &mmsz, &mmver);
-               if (EFI_ERROR(status)) {
-                       printf("%s: GetMemoryMap error %lu\n", __func__,
-                           EFI_ERROR_CODE(status));
-                       return (EINVAL);
-               }
-               status = BS->ExitBootServices(IH, efi_mapkey);
-               if (EFI_ERROR(status) == 0) {
+                       if (addr != 0)
+                               BS->FreePages(addr, pages);
+
+                       /* Add 10 descriptors to the size to allow for
+                        * fragmentation caused by calling AllocatePages */
+                       sz += (10 * dsz);
+                       pages = EFI_SIZE_TO_PAGES(sz + efisz);
+                       status = BS->AllocatePages(AllocateAnyPages, 
EfiLoaderData,
+                                       pages, &addr);
+                       if (EFI_ERROR(status)) {
+                               printf("%s: AllocatePages error %lu\n", 
__func__,
+                                   EFI_ERROR_CODE(status));
+                               return (ENOMEM);
+                       }
+
                        /*
-                        * This may be disabled by setting efi_disable_vmap in
-                        * loader.conf(5). By default we will setup the virtual
-                        * map entries.
+                        * Read the memory map and stash it after bootinfo. 
Align the
+                        * memory map on a 16-byte boundary (the bootinfo block 
is page
+                        * aligned).
                         */
-                       if (do_vmap)
-                               efi_do_vmap(mm, sz, mmsz, mmver);
-                       efihdr->memory_size = sz;
-                       efihdr->descriptor_size = mmsz;
-                       efihdr->descriptor_version = mmver;
-                       file_addmetadata(kfp, MODINFOMD_EFI_MAP, efisz + sz,
-                           efihdr);
-                       return (0);
+                       efihdr = (struct efi_map_header *)(uintptr_t)addr;
+                       mm = (void *)((uint8_t *)efihdr + efisz);
+                       sz = (EFI_PAGE_SIZE * pages) - efisz;
                }
+
+               status = BS->ExitBootServices(IH, efi_mapkey);
+               if (!EFI_ERROR(status))
+                       break;
+       }
+
+       if (retry == 0) {
                BS->FreePages(addr, pages);
+               printf("ExitBootServices error %lu\n", EFI_ERROR_CODE(status));
+               return (EINVAL);
        }
-       printf("ExitBootServices error %lu\n", EFI_ERROR_CODE(status));
-       return (EINVAL);
+
+       /*
+        * This may be disabled by setting efi_disable_vmap in
+        * loader.conf(5). By default we will setup the virtual
+        * map entries.
+        */
+
+       if (do_vmap)
+               efi_do_vmap(mm, sz, dsz, mmver);
+       efihdr->memory_size = sz;
+       efihdr->descriptor_size = dsz;
+       efihdr->descriptor_version = mmver;
+       file_addmetadata(kfp, MODINFOMD_EFI_MAP, efisz + sz,
+           efihdr);
+
+       return (0);
 }
 
 /*

Modified: stable/12/stand/efi/loader/copy.c
==============================================================================
--- stable/12/stand/efi/loader/copy.c   Fri Sep 27 02:09:20 2019        
(r352787)
+++ stable/12/stand/efi/loader/copy.c   Fri Sep 27 05:12:28 2019        
(r352788)
@@ -95,7 +95,7 @@ static void
 efi_verify_staging_size(unsigned long *nr_pages)
 {
        UINTN sz;
-       EFI_MEMORY_DESCRIPTOR *map, *p;
+       EFI_MEMORY_DESCRIPTOR *map = NULL, *p;
        EFI_PHYSICAL_ADDRESS start, end;
        UINTN key, dsz;
        UINT32 dver;
@@ -104,17 +104,28 @@ efi_verify_staging_size(unsigned long *nr_pages)
        unsigned long available_pages = 0;
 
        sz = 0;
-       status = BS->GetMemoryMap(&sz, 0, &key, &dsz, &dver);
-       if (status != EFI_BUFFER_TOO_SMALL) {
-               printf("Can't determine memory map size\n");
-               return;
-       }
 
-       map = malloc(sz);
-       status = BS->GetMemoryMap(&sz, map, &key, &dsz, &dver);
-       if (EFI_ERROR(status)) {
-               printf("Can't read memory map\n");
-               goto out;
+       for (;;) {
+               status = BS->GetMemoryMap(&sz, map, &key, &dsz, &dver);
+               if (!EFI_ERROR(status))
+                       break;
+
+               if (status != EFI_BUFFER_TOO_SMALL) {
+                       printf("Can't read memory map: %lu\n",
+                           EFI_ERROR_CODE(status));
+                       goto out;
+               }
+
+               free(map);
+
+               /* Allocate 10 descriptors more than the size reported,
+                * to allow for any fragmentation caused by calling
+                * malloc */
+               map = malloc(sz + (10 * dsz));
+               if (map == NULL) {
+                       printf("Unable to allocate memory\n");
+                       goto out;
+               }
        }
 
        ndesc = sz / dsz;
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to