All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] grub efi memory map patch
@ 2006-10-24  6:58 bibo,mao
  2006-10-24 13:26 ` Johan Rydberg
  0 siblings, 1 reply; 3+ messages in thread
From: bibo,mao @ 2006-10-24  6:58 UTC (permalink / raw)
  To: grub-devel

This patch moves find_mmap_size from i386/efi/linux.c to
kern/efi/mm.c, and renamed as grub_efi_find_mmap_size, so that
other arch on EFI platform can use this function.

Also this function solves memory map too small problem,
on some EFI platform MEMORY_MAP_SIZE(0x1000) is a little smaller
than EFI memory map table.

any suggestion is welcome.

thanks
bibo,mao


diff -Nruap grub2.org/include/grub/efi/api.h grub2/include/grub/efi/api.h
--- grub2.org/include/grub/efi/api.h	2006-10-24 13:05:48.000000000 +0800
+++ grub2/include/grub/efi/api.h	2006-10-23 14:35:12.000000000 +0800
@@ -206,6 +206,13 @@ typedef grub_efi_intn_t grub_efi_status_
 #define GRUB_EFI_WARN_WRITE_FAILURE	GRUB_EFI_WARNING_CODE (3)
 #define GRUB_EFI_WARN_BUFFER_TOO_SMALL	GRUB_EFI_WARNING_CODE (4)
 
+#define GRUB_EFI_PAGE_SHIFT		12
+#define GRUB_EFI_PAGE_SIZE 		(0x1UL << GRUB_EFI_PAGE_SHIFT)
+#define GRUB_EFI_PAGES(addr) 		(addr >> GRUB_EFI_PAGE_SHIFT)
+#define GRUB_EFI_PAGES_UP(addr) 	((addr + GRUB_EFI_PAGE_SIZE - 1) >> GRUB_EFI_PAGE_SHIFT)
+#define PAGE_DOWN(addr) 		((addr) & (~(GRUB_EFI_PAGE_SIZE - 1)))
+#define PAGE_UP(addr)   		PAGE_DOWN(addr + GRUB_EFI_PAGE_SIZE - 1)
+
 typedef void *grub_efi_handle_t;
 typedef void *grub_efi_event_t;
 typedef grub_efi_uint64_t grub_efi_lba_t;
diff -Nruap grub2.org/include/grub/efi/efi.h grub2/include/grub/efi/efi.h
--- grub2.org/include/grub/efi/efi.h	2006-10-24 13:05:48.000000000 +0800
+++ grub2/include/grub/efi/efi.h	2006-10-23 14:42:48.000000000 +0800
@@ -55,6 +55,7 @@ char *EXPORT_FUNC(grub_efi_get_filename)
 grub_efi_device_path_t *
 EXPORT_FUNC(grub_efi_get_device_path) (grub_efi_handle_t handle);
 int EXPORT_FUNC(grub_efi_exit_boot_services) (grub_efi_uintn_t map_key);
+grub_efi_uintn_t EXPORT_FUNC(grub_efi_find_mmap_size) (void);
 
 void grub_efi_mm_init (void);
 void grub_efi_mm_fini (void);
diff -Nruap grub2.org/kern/efi/mm.c grub2/kern/efi/mm.c
--- grub2.org/kern/efi/mm.c	2006-10-24 13:05:48.000000000 +0800
+++ grub2/kern/efi/mm.c	2006-10-24 13:18:48.000000000 +0800
@@ -30,10 +30,6 @@
 #define BYTES_TO_PAGES(bytes)	((bytes) >> 12)
 #define PAGES_TO_BYTES(pages)	((pages) << 12)
 
-/* The size of a memory map obtained from the firmware. This must be
-   a multiplier of 4KB.  */
-#define MEMORY_MAP_SIZE	0x1000
-
 /* Maintain the list of allocated pages.  */
 struct allocated_page
 {
@@ -335,6 +331,45 @@ print_memory_map (grub_efi_memory_descri
 }
 #endif
 
+/* Find the optimal number of pages for the memory map. */
+grub_efi_uintn_t
+grub_efi_find_mmap_size (void)
+{     
+  static grub_efi_uintn_t mmap_size = 0;
+    
+  if (mmap_size != 0)
+    return mmap_size;
+    
+  mmap_size = GRUB_EFI_PAGE_SIZE;
+  while (1)
+    {
+      int ret;
+      grub_efi_memory_descriptor_t *mmap;
+      grub_efi_uintn_t desc_size;
+      
+      mmap = grub_efi_allocate_pages ( 0, GRUB_EFI_PAGES_UP(mmap_size));
+      if (! mmap)
+        return 0;
+ 
+      ret = grub_efi_get_memory_map (&mmap_size, mmap, 0, &desc_size, 0);
+      grub_efi_free_pages ((grub_addr_t) mmap, GRUB_EFI_PAGES_UP(mmap_size));
+       
+      if (ret < 0)
+        grub_fatal ("cannot get memory map");
+      else if (ret > 0)
+        break;
+ 
+      mmap_size += GRUB_EFI_PAGE_SIZE;
+    }
+ 
+  /* Increase the size a bit for safety, because GRUB allocates more on
+     later, and EFI itself may allocate more.  */
+  mmap_size += GRUB_EFI_PAGE_SIZE;
+  mmap_size = PAGE_UP(mmap_size);
+ 
+  return mmap_size;
+}
+
 void
 grub_efi_mm_init (void)
 {
@@ -346,6 +381,8 @@ grub_efi_mm_init (void)
   grub_efi_uintn_t desc_size;
   grub_efi_uint64_t total_pages;
   grub_efi_uint64_t required_pages;
+  grub_efi_uintn_t memory_map_size;
+  int res;
 
   /* First of all, allocate pages to maintain allocations.  */
   allocated_pages
@@ -355,16 +392,20 @@ grub_efi_mm_init (void)
 
   grub_memset (allocated_pages, 0, ALLOCATED_PAGES_SIZE);
   
-  /* Prepare a memory region to store two memory maps.  */
-  memory_map = grub_efi_allocate_pages (0,
-					2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
+  /* Prepare a memory region to store two memory maps.  Obtain size of
+     memory map by passing a NULL buffer and a buffer size of
+     zero.  */
+  memory_map_size = grub_efi_find_mmap_size( );
+  memory_map = grub_efi_allocate_pages(0, 2 * PAGE_UP(memory_map_size));
+
   if (! memory_map)
     grub_fatal ("cannot allocate memory");
 
-  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map, MEMORY_MAP_SIZE);
+  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map, 
+						PAGE_UP(memory_map_size));
 
   /* Obtain descriptors for available memory.  */
-  map_size = MEMORY_MAP_SIZE;
+  map_size = memory_map_size;
 
   if (grub_efi_get_memory_map (&map_size, memory_map, 0, &desc_size, 0) < 0)
     grub_fatal ("cannot get memory map");
@@ -405,8 +446,7 @@ grub_efi_mm_init (void)
 #endif
   
   /* Release the memory maps.  */
-  grub_efi_free_pages ((grub_addr_t) memory_map,
-		       2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
+  grub_efi_free_pages ((grub_addr_t) memory_map, 2 * PAGE_UP(memory_map_size));
 }
 
 void
diff -Nruap grub2.org/loader/i386/efi/linux.c grub2/loader/i386/efi/linux.c
--- grub2.org/loader/i386/efi/linux.c	2006-10-24 13:06:56.000000000 +0800
+++ grub2/loader/i386/efi/linux.c	2006-10-23 14:42:48.000000000 +0800
@@ -93,45 +93,6 @@ page_align (grub_size_t size)
   return (size + (1 << 12) - 1) & (~((1 << 12) - 1));
 }
 
-/* Find the optimal number of pages for the memory map. Is it better to
-   move this code to efi/mm.c?  */
-static grub_efi_uintn_t
-find_mmap_size (void)
-{
-  static grub_efi_uintn_t mmap_size = 0;
-
-  if (mmap_size != 0)
-    return mmap_size;
-  
-  mmap_size = (1 << 12);
-  while (1)
-    {
-      int ret;
-      grub_efi_memory_descriptor_t *mmap;
-      grub_efi_uintn_t desc_size;
-      
-      mmap = grub_malloc (mmap_size);
-      if (! mmap)
-	return 0;
-
-      ret = grub_efi_get_memory_map (&mmap_size, mmap, 0, &desc_size, 0);
-      grub_free (mmap);
-      
-      if (ret < 0)
-	grub_fatal ("cannot get memory map");
-      else if (ret > 0)
-	break;
-
-      mmap_size += (1 << 12);
-    }
-
-  /* Increase the size a bit for safety, because GRUB allocates more on
-     later, and EFI itself may allocate more.  */
-  mmap_size += (1 << 12);
-
-  return page_align (mmap_size);
-}
-
 static void
 free_pages (void)
 {
@@ -167,7 +128,7 @@ allocate_pages (grub_size_t real_size, g
   /* Make sure that each size is aligned to a page boundary.  */
   real_size = page_align (real_size + GRUB_DISK_SECTOR_SIZE);
   prot_size = page_align (prot_size);
-  mmap_size = find_mmap_size ();
+  mmap_size = grub_efi_find_mmap_size ();
 
   grub_dprintf ("linux", "real_size = %x, prot_size = %x, mmap_size = %x\n",
 		real_size, prot_size, mmap_size);
@@ -275,7 +236,7 @@ grub_linux_boot (void)
   grub_dprintf ("linux", "idt = %x:%x, gdt = %x:%x\n",
 		(unsigned) idt_desc.limit, (unsigned) idt_desc.base,
 		(unsigned) gdt_desc.limit, (unsigned) gdt_desc.base);
-  mmap_size = find_mmap_size ();
+  mmap_size = grub_efi_find_mmap_size ();
   if (grub_efi_get_memory_map (&mmap_size, mmap_buf, &map_key,
 			       &desc_size, &desc_version) <= 0)
     grub_fatal ("cannot get memory map");
@@ -616,7 +577,7 @@ grub_rescue_cmd_initrd (int argc, char *
   addr_min = (grub_addr_t) prot_mode_mem + ((prot_mode_pages * 3) << 12);
   
   /* Find the highest address to put the initrd.  */
-  mmap_size = find_mmap_size ();
+  mmap_size = grub_efi_find_mmap_size ();
   if (grub_efi_get_memory_map (&mmap_size, mmap_buf, 0, &desc_size, 0) <= 0)
     grub_fatal ("cannot get memory map");
 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/3] grub efi memory map patch
  2006-10-24  6:58 [PATCH 1/3] grub efi memory map patch bibo,mao
@ 2006-10-24 13:26 ` Johan Rydberg
  2006-10-25  5:38   ` bibo,mao
  0 siblings, 1 reply; 3+ messages in thread
From: Johan Rydberg @ 2006-10-24 13:26 UTC (permalink / raw)
  To: The development of GRUB 2

[-- Attachment #1: Type: text/plain, Size: 4616 bytes --]

"bibo,mao" <bibo.mao@intel.com> writes:

> This patch moves find_mmap_size from i386/efi/linux.c to
> kern/efi/mm.c, and renamed as grub_efi_find_mmap_size, so that
> other arch on EFI platform can use this function.

Good call.

> Also this function solves memory map too small problem,
> on some EFI platform MEMORY_MAP_SIZE(0x1000) is a little smaller
> than EFI memory map table.
>
> any suggestion is welcome.

Just a few comments here.  I will leave a full review to Okuji.

> +#define GRUB_EFI_PAGE_SHIFT		12
> +#define GRUB_EFI_PAGE_SIZE 		(0x1UL << GRUB_EFI_PAGE_SHIFT)
> +#define GRUB_EFI_PAGES(addr) 		(addr >> GRUB_EFI_PAGE_SHIFT)
> +#define GRUB_EFI_PAGES_UP(addr) 	((addr + GRUB_EFI_PAGE_SIZE - 1) >> GRUB_EFI_PAGE_SHIFT)
> +#define PAGE_DOWN(addr) 		((addr) & (~(GRUB_EFI_PAGE_SIZE - 1)))
> +#define PAGE_UP(addr)   		PAGE_DOWN(addr + GRUB_EFI_PAGE_SIZE - 1)

The PAGE_DOWN and PAGE_UP macros should be named something else, with
a GRUB_EFI prefix.  What about GRUB_EFI_PAGE_TRUNC and
GRUB_EFI_PAGE_ROUND?  Not sure GRUB_EFI_PAGES_UP is needed, since
GRUB_EFI_PAGES (GRUB_EFI_PAGE_ROUND (value)) can be used instead.  If
you find it too long, add a local macro to the file where you use it.

Also, please slap a few parenthesis round marco arguments.

> +/* Find the optimal number of pages for the memory map. */
> +grub_efi_uintn_t
> +grub_efi_find_mmap_size (void)
> +{     +  static grub_efi_uintn_t mmap_size = 0;
> +    +  if (mmap_size != 0)
> +    return mmap_size;
> +    +  mmap_size = GRUB_EFI_PAGE_SIZE;
> +  while (1)
> +    {
> +      int ret;
> +      grub_efi_memory_descriptor_t *mmap;
> +      grub_efi_uintn_t desc_size;
> +      +      mmap = grub_efi_allocate_pages ( 0,
> GRUB_EFI_PAGES_UP(mmap_size));
> +      if (! mmap)
> +        return 0;
> + +      ret = grub_efi_get_memory_map (&mmap_size, mmap, 0,
> &desc_size, 0);
> +      grub_efi_free_pages ((grub_addr_t) mmap, GRUB_EFI_PAGES_UP(mmap_size));
> +       +      if (ret < 0)
> +        grub_fatal ("cannot get memory map");
> +      else if (ret > 0)
> +        break;
> + +      mmap_size += GRUB_EFI_PAGE_SIZE;
> +    }
> + +  /* Increase the size a bit for safety, because GRUB allocates
> more on
> +     later, and EFI itself may allocate more.  */
> +  mmap_size += GRUB_EFI_PAGE_SIZE;
> +  mmap_size = PAGE_UP(mmap_size);
> + +  return mmap_size;
> +}

Is this some patching gone wrong?  What are all the "+"'s doing there?

The specification states that GetMemoryMap will return
EFI_BUFFER_TOO_SMALL and the memory map size in *MemoryMapSize if the
memory map could not fit in the buffer (which may be NULL if provided
buffer size is too small).  Meaning that you can get the size of the
memory map by simply doing;

grub_efi_uintn_t
grub_efi_find_mmap_size (void)
{
  mmap_size = 0;
  err = grub_efi_get_memory_map (&mmap_size, NULL, 0, NULL, 0);
  if (err != GRUB_EFI_BUFFER_TOO_SMALL)
    return err;
  return mmap_size;
}

(code not tested, nor compiled.)

I think Tristan did this in his patch that addresses the same problem
of that the memory map does not fit in a single page.

> +
> void
> grub_efi_mm_init (void)

Whitespace changes are normally not welcome.  But they could be
removed by the maintainer who commits it.  No worries.

B> {
> @@ -346,6 +381,8 @@ grub_efi_mm_init (void)
>   grub_efi_uintn_t desc_size;
>   grub_efi_uint64_t total_pages;
>   grub_efi_uint64_t required_pages;
> +  grub_efi_uintn_t memory_map_size;
> +  int res;
>
>   /* First of all, allocate pages to maintain allocations.  */
>   allocated_pages
> @@ -355,16 +392,20 @@ grub_efi_mm_init (void)
>
>   grub_memset (allocated_pages, 0, ALLOCATED_PAGES_SIZE);
>   -  /* Prepare a memory region to store two memory maps.  */
> -  memory_map = grub_efi_allocate_pages (0,
> -					2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
> +  /* Prepare a memory region to store two memory maps.  Obtain size of
> +     memory map by passing a NULL buffer and a buffer size of
> +     zero.  */
> +  memory_map_size = grub_efi_find_mmap_size( );
> +  memory_map = grub_efi_allocate_pages(0, 2 * PAGE_UP(memory_map_size));
> +

One space between function name and parenthesis, and no spaces within
a empty argument list.

>   if (! memory_map)
>     grub_fatal ("cannot allocate memory");
>
> -  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map, MEMORY_MAP_SIZE);
> +  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map,
> +
> PAGE_UP(memory_map_size));

I suppose this is a line-wrapped patch? 


[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/3] grub efi memory map patch
  2006-10-24 13:26 ` Johan Rydberg
@ 2006-10-25  5:38   ` bibo,mao
  0 siblings, 0 replies; 3+ messages in thread
From: bibo,mao @ 2006-10-25  5:38 UTC (permalink / raw)
  To: The development of GRUB 2

hi johan, 
   Thanks for review my patch, I reply in lines.

Johan Rydberg wrote:
> "bibo,mao" <bibo.mao@intel.com> writes:
> 
>  > This patch moves find_mmap_size from i386/efi/linux.c to
>  > kern/efi/mm.c, and renamed as grub_efi_find_mmap_size, so that
>  > other arch on EFI platform can use this function.
> 
> Good call.
> 
>  > Also this function solves memory map too small problem,
>  > on some EFI platform MEMORY_MAP_SIZE(0x1000) is a little smaller
>  > than EFI memory map table.
>  >
>  > any suggestion is welcome.
> 
> Just a few comments here.  I will leave a full review to Okuji.
> 
>  > +#define GRUB_EFI_PAGE_SHIFT          12
>  > +#define GRUB_EFI_PAGE_SIZE           (0x1UL << GRUB_EFI_PAGE_SHIFT)
>  > +#define GRUB_EFI_PAGES(addr)                 (addr >> GRUB_EFI_PAGE_SHIFT)
>  > +#define GRUB_EFI_PAGES_UP(addr)      ((addr + GRUB_EFI_PAGE_SIZE - 1) >> 
> GRUB_EFI_PAGE_SHIFT)
>  > +#define PAGE_DOWN(addr)              ((addr) & (~(GRUB_EFI_PAGE_SIZE - 1)))
>  > +#define PAGE_UP(addr)                PAGE_DOWN(addr + GRUB_EFI_PAGE_SIZE - 1)
> 
> The PAGE_DOWN and PAGE_UP macros should be named something else, with
> a GRUB_EFI prefix.  What about GRUB_EFI_PAGE_TRUNC and
> GRUB_EFI_PAGE_ROUND?  Not sure GRUB_EFI_PAGES_UP is needed, since
> GRUB_EFI_PAGES (GRUB_EFI_PAGE_ROUND (value)) can be used instead.  If
> you find it too long, add a local macro to the file where you use it.

It sounds better for me, I rename the macro name as GRUB_EFI_PAGE_TRUNC
and GRUB_EFI_PAGE_ROUND, and delete GRUB_EFI_PAGES_UP macro.

> Also, please slap a few parenthesis round marco arguments.
> 
>  > +/* Find the optimal number of pages for the memory map. */
>  > +grub_efi_uintn_t
>  > +grub_efi_find_mmap_size (void)
>  > +{     +  static grub_efi_uintn_t mmap_size = 0;
>  > +    +  if (mmap_size != 0)
>  > +    return mmap_size;
>  > +    +  mmap_size = GRUB_EFI_PAGE_SIZE;
>  > +  while (1)
>  > +    {
>  > +      int ret;
>  > +      grub_efi_memory_descriptor_t *mmap;
>  > +      grub_efi_uintn_t desc_size;
>  > +      +      mmap = grub_efi_allocate_pages ( 0,
>  > GRUB_EFI_PAGES_UP(mmap_size));
>  > +      if (! mmap)
>  > +        return 0;
>  > + +      ret = grub_efi_get_memory_map (&mmap_size, mmap, 0,
>  > &desc_size, 0);
>  > +      grub_efi_free_pages ((grub_addr_t) mmap, GRUB_EFI_PAGES_UP(mmap_size));
>  > +       +      if (ret < 0)
>  > +        grub_fatal ("cannot get memory map");
>  > +      else if (ret > 0)
>  > +        break;
>  > + +      mmap_size += GRUB_EFI_PAGE_SIZE;
>  > +    }
>  > + +  /* Increase the size a bit for safety, because GRUB allocates
>  > more on
>  > +     later, and EFI itself may allocate more.  */
>  > +  mmap_size += GRUB_EFI_PAGE_SIZE;
>  > +  mmap_size = PAGE_UP(mmap_size);
>  > + +  return mmap_size;
>  > +}
> 
> Is this some patching gone wrong?  What are all the "+"'s doing there?
> 
> The specification states that GetMemoryMap will return
> EFI_BUFFER_TOO_SMALL and the memory map size in *MemoryMapSize if the
> memory map could not fit in the buffer (which may be NULL if provided
> buffer size is too small).  Meaning that you can get the size of the
> memory map by simply doing;
> 
> grub_efi_uintn_t
> grub_efi_find_mmap_size (void)
> {
>   mmap_size = 0;
>   err = grub_efi_get_memory_map (&mmap_size, NULL, 0, NULL, 0);
>   if (err != GRUB_EFI_BUFFER_TOO_SMALL)
>     return err;
>   return mmap_size;
> }
> 
> (code not tested, nor compiled.)
> 
> I think Tristan did this in his patch that addresses the same problem
> of that the memory map does not fit in a single page.
Yes, it is better. But there is one point reserved to notice, 
grub_efi_find_mmap_size will return current memory map size, if there is
memory alloc function, map size will be larger. so allocated 
(mmap_size + one extra page) buffer for memory map buffer is more reasonable.

>  > +
>  > void
>  > grub_efi_mm_init (void)
> 
> Whitespace changes are normally not welcome.  But they could be
> removed by the maintainer who commits it.  No worries.
> 
> B> {
>  > @@ -346,6 +381,8 @@ grub_efi_mm_init (void)
>  >   grub_efi_uintn_t desc_size;
>  >   grub_efi_uint64_t total_pages;
>  >   grub_efi_uint64_t required_pages;
>  > +  grub_efi_uintn_t memory_map_size;
>  > +  int res;
>  >
>  >   /* First of all, allocate pages to maintain allocations.  */
>  >   allocated_pages
>  > @@ -355,16 +392,20 @@ grub_efi_mm_init (void)
>  >
>  >   grub_memset (allocated_pages, 0, ALLOCATED_PAGES_SIZE);
>  >   -  /* Prepare a memory region to store two memory maps.  */
>  > -  memory_map = grub_efi_allocate_pages (0,
>  > -                                     2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
>  > +  /* Prepare a memory region to store two memory maps.  Obtain size of
>  > +     memory map by passing a NULL buffer and a buffer size of
>  > +     zero.  */
>  > +  memory_map_size = grub_efi_find_mmap_size( );
>  > +  memory_map = grub_efi_allocate_pages(0, 2 * PAGE_UP(memory_map_size));
>  > +
> 
> One space between function name and parenthesis, and no spaces within
> a empty argument list.
I am not familiar with GNU coding style:(
The second patch corrects the point this time.
> 
>  >   if (! memory_map)
>  >     grub_fatal ("cannot allocate memory");
>  >
>  > -  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map, MEMORY_MAP_SIZE);
>  > +  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map,
>  > +
>  > PAGE_UP(memory_map_size));
> 
> I suppose this is a line-wrapped patch?
> 
> 

diff -Nrup grub2.org/include/grub/efi/api.h grub2/include/grub/efi/api.h
--- grub2.org/include/grub/efi/api.h	2006-10-25 12:16:38.000000000 +0800
+++ grub2/include/grub/efi/api.h	2006-10-25 11:01:32.000000000 +0800
@@ -206,6 +206,12 @@ typedef grub_efi_intn_t grub_efi_status_
 #define GRUB_EFI_WARN_WRITE_FAILURE	GRUB_EFI_WARNING_CODE (3)
 #define GRUB_EFI_WARN_BUFFER_TOO_SMALL	GRUB_EFI_WARNING_CODE (4)
 
+#define GRUB_EFI_PAGE_SHIFT		12
+#define GRUB_EFI_PAGE_SIZE 		(0x1UL << GRUB_EFI_PAGE_SHIFT)
+#define GRUB_EFI_PAGES(addr) 		(addr >> GRUB_EFI_PAGE_SHIFT)
+#define GRUB_EFI_PAGE_TRUNC(addr)	((addr) & (~(GRUB_EFI_PAGE_SIZE - 1)))
+#define GRUB_EFI_PAGE_ROUND(addr)	GRUB_EFI_PAGE_TRUNC(addr + GRUB_EFI_PAGE_SIZE - 1)
+
 typedef void *grub_efi_handle_t;
 typedef void *grub_efi_event_t;
 typedef grub_efi_uint64_t grub_efi_lba_t;
diff -Nrup grub2.org/include/grub/efi/efi.h grub2/include/grub/efi/efi.h
--- grub2.org/include/grub/efi/efi.h	2006-10-25 12:16:38.000000000 +0800
+++ grub2/include/grub/efi/efi.h	2006-10-25 10:53:04.000000000 +0800
@@ -55,6 +55,7 @@ char *EXPORT_FUNC(grub_efi_get_filename)
 grub_efi_device_path_t *
 EXPORT_FUNC(grub_efi_get_device_path) (grub_efi_handle_t handle);
 int EXPORT_FUNC(grub_efi_exit_boot_services) (grub_efi_uintn_t map_key);
+grub_efi_uintn_t EXPORT_FUNC(grub_efi_find_mmap_size) (void);
 
 void grub_efi_mm_init (void);
 void grub_efi_mm_fini (void);
diff -Nrup grub2.org/kern/efi/mm.c grub2/kern/efi/mm.c
--- grub2.org/kern/efi/mm.c	2006-10-25 12:16:38.000000000 +0800
+++ grub2/kern/efi/mm.c	2006-10-25 11:21:10.000000000 +0800
@@ -30,10 +30,6 @@
 #define BYTES_TO_PAGES(bytes)	((bytes) >> 12)
 #define PAGES_TO_BYTES(pages)	((pages) << 12)
 
-/* The size of a memory map obtained from the firmware. This must be
-   a multiplier of 4KB.  */
-#define MEMORY_MAP_SIZE	0x1000
-
 /* Maintain the list of allocated pages.  */
 struct allocated_page
 {
@@ -335,6 +331,21 @@ print_memory_map (grub_efi_memory_descri
 }
 #endif
 
+/* Find the optimal number of pages for the memory map. */
+grub_efi_uintn_t
+grub_efi_find_mmap_size(void)
+{     
+  grub_efi_uintn_t mmap_size = 0;
+  grub_efi_uintn_t desc_size;
+  int ret;
+
+  ret = grub_efi_get_memory_map (&mmap_size, NULL, 0, &desc_size, 0);
+  if (ret < 0)
+    grub_fatal ("cannot get memory map");
+    
+  return mmap_size;
+}
+
 void
 grub_efi_mm_init (void)
 {
@@ -346,6 +357,8 @@ grub_efi_mm_init (void)
   grub_efi_uintn_t desc_size;
   grub_efi_uint64_t total_pages;
   grub_efi_uint64_t required_pages;
+  grub_efi_uintn_t memory_map_size;
+  int res;
 
   /* First of all, allocate pages to maintain allocations.  */
   allocated_pages
@@ -355,16 +368,25 @@ grub_efi_mm_init (void)
 
   grub_memset (allocated_pages, 0, ALLOCATED_PAGES_SIZE);
   
-  /* Prepare a memory region to store two memory maps.  */
-  memory_map = grub_efi_allocate_pages (0,
-					2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
+  /* Prepare a memory region to store two memory maps.  Obtain size of
+     memory map by passing a NULL buffer and a buffer size of
+     zero.  */
+  memory_map_size = grub_efi_find_mmap_size();
+
+  /* there will be memory alloc after getting memory_map_size
+   * so here GRUB_EFI_PAGE_SIZE is added.  */ 
+  memory_map_size += GRUB_EFI_PAGE_SIZE; 
+  memory_map = grub_efi_allocate_pages (0, 
+				2 * GRUB_EFI_PAGE_ROUND (memory_map_size));
+
   if (! memory_map)
     grub_fatal ("cannot allocate memory");
 
-  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map, MEMORY_MAP_SIZE);
+  filtered_memory_map = NEXT_MEMORY_DESCRIPTOR (memory_map, 
+					GRUB_EFI_PAGE_ROUND (memory_map_size));
 
   /* Obtain descriptors for available memory.  */
-  map_size = MEMORY_MAP_SIZE;
+  map_size = memory_map_size;
 
   if (grub_efi_get_memory_map (&map_size, memory_map, 0, &desc_size, 0) < 0)
     grub_fatal ("cannot get memory map");
@@ -406,7 +428,7 @@ grub_efi_mm_init (void)
   
   /* Release the memory maps.  */
   grub_efi_free_pages ((grub_addr_t) memory_map,
-		       2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
+			 2 * GRUB_EFI_PAGE_ROUND (memory_map_size));
 }
 
 void
diff -Nrup grub2.org/loader/i386/efi/linux.c grub2/loader/i386/efi/linux.c
--- grub2.org/loader/i386/efi/linux.c	2006-10-25 12:16:45.000000000 +0800
+++ grub2/loader/i386/efi/linux.c	2006-10-25 11:40:24.000000000 +0800
@@ -93,45 +93,6 @@ page_align (grub_size_t size)
   return (size + (1 << 12) - 1) & (~((1 << 12) - 1));
 }
 
-/* Find the optimal number of pages for the memory map. Is it better to
-   move this code to efi/mm.c?  */
-static grub_efi_uintn_t
-find_mmap_size (void)
-{
-  static grub_efi_uintn_t mmap_size = 0;
-
-  if (mmap_size != 0)
-    return mmap_size;
-  
-  mmap_size = (1 << 12);
-  while (1)
-    {
-      int ret;
-      grub_efi_memory_descriptor_t *mmap;
-      grub_efi_uintn_t desc_size;
-      
-      mmap = grub_malloc (mmap_size);
-      if (! mmap)
-	return 0;
-
-      ret = grub_efi_get_memory_map (&mmap_size, mmap, 0, &desc_size, 0);
-      grub_free (mmap);
-      
-      if (ret < 0)
-	grub_fatal ("cannot get memory map");
-      else if (ret > 0)
-	break;
-
-      mmap_size += (1 << 12);
-    }
-
-  /* Increase the size a bit for safety, because GRUB allocates more on
-     later, and EFI itself may allocate more.  */
-  mmap_size += (1 << 12);
-
-  return page_align (mmap_size);
-}
-
 static void
 free_pages (void)
 {
@@ -167,14 +128,14 @@ allocate_pages (grub_size_t real_size, g
   /* Make sure that each size is aligned to a page boundary.  */
   real_size = page_align (real_size + GRUB_DISK_SECTOR_SIZE);
   prot_size = page_align (prot_size);
-  mmap_size = find_mmap_size ();
+  mmap_size = grub_efi_find_mmap_size();
 
   grub_dprintf ("linux", "real_size = %x, prot_size = %x, mmap_size = %x\n",
 		real_size, prot_size, mmap_size);
   
   /* Calculate the number of pages; Combine the real mode code with
      the memory map buffer for simplicity.  */
-  real_mode_pages = ((real_size + mmap_size) >> 12);
+  real_mode_pages = ((real_size + mmap_size + GRUB_EFI_PAGE_SIZE) >> 12);
   prot_mode_pages = (prot_size >> 12);
   
   /* Initialize the memory pointers with NULL for convenience.  */
@@ -214,7 +175,7 @@ allocate_pages (grub_size_t real_size, g
 	  grub_dprintf ("linux", "physical_start = %x, physical_end = %x\n",
 			(unsigned) desc->physical_start,
 			(unsigned) physical_end);
-	  addr = physical_end - real_size - mmap_size;
+	  addr = physical_end - (real_mode_pages << GRUB_EFI_PAGE_SHIFT);
 	  if (addr < 0x10000)
 	    continue;
 
@@ -275,7 +236,7 @@ grub_linux_boot (void)
   grub_dprintf ("linux", "idt = %x:%x, gdt = %x:%x\n",
 		(unsigned) idt_desc.limit, (unsigned) idt_desc.base,
 		(unsigned) gdt_desc.limit, (unsigned) gdt_desc.base);
-  mmap_size = find_mmap_size ();
+  mmap_size = grub_efi_find_mmap_size();
   if (grub_efi_get_memory_map (&mmap_size, mmap_buf, &map_key,
 			       &desc_size, &desc_version) <= 0)
     grub_fatal ("cannot get memory map");
@@ -616,7 +577,7 @@ grub_rescue_cmd_initrd (int argc, char *
   addr_min = (grub_addr_t) prot_mode_mem + ((prot_mode_pages * 3) << 12);
   
   /* Find the highest address to put the initrd.  */
-  mmap_size = find_mmap_size ();
+  mmap_size = grub_efi_find_mmap_size();
   if (grub_efi_get_memory_map (&mmap_size, mmap_buf, 0, &desc_size, 0) <= 0)
     grub_fatal ("cannot get memory map");
 

 



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2006-10-25  5:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-24  6:58 [PATCH 1/3] grub efi memory map patch bibo,mao
2006-10-24 13:26 ` Johan Rydberg
2006-10-25  5:38   ` bibo,mao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.