Multiboot memory detection fixes
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 15 Aug 2013 00:17:39 +0000 (17:17 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 15 Aug 2013 00:17:39 +0000 (17:17 -0700)
A few things:
- Multiboot's regions don't cover all of physical memory; it has holes.  We now
  init the pages[] array (for x86) to mark all pages as busy, and explicitly
  mark the free pages.
- The previous version of this worked a little, since the unmarked, non-free
  pages weren't on any free page lists.  However, the contiguous memory
  allocator (which is really lousy btw) looks directly at refcnts, bypassing
  the lists.
- One problem is that there is a big chunk of unusable memory around 0xfec00000
  (IOAPIC and friends).  This was working okay with multiboot's memory
  mappings, but not with qemu's "here's a lot of memory" approach.  In lieu of
  doing full-fledged memory detection/probing, I'm just ignoring physical
  memory from [0xc0000000, 0xffffffff].  This only applies to qemu, when
  launched with the -kernel flag.
- 32 bit had some other issues when giving out memory near the 4GB mark, as
  well as having other issues with 64 bit addresses (note some of the changes
  from %p to 0x%016llx).
- The bug with giving out pages that were mapped to IO/BIOS/x86 magic regions
  in high memory showed up as clobbered bookkeeping in small slab pages in
  kmem_cache_grow() (stored at the end of the pages improperly allocated).
  Good times!

kern/arch/x86/page_alloc.c
kern/arch/x86/ros/mmu32.h
kern/src/multiboot.c

index 4a3d7e8..44d84c6 100644 (file)
@@ -39,6 +39,8 @@ static void track_free_page(struct page *page)
                                                                llc_cache)],
                         page, pg_link);
        nr_free_pages++;
+       /* Page was previous marked as busy, need to set it free explicitly */
+       page_setref(page, 0);
 }
 
 static struct page *pa64_to_page(uint64_t paddr)
@@ -54,7 +56,8 @@ static bool pa64_is_in_kernel(uint64_t paddr)
 }
 
 /* Helper.  For every page in the entry, this will determine whether or not the
- * page is free, and handle accordingly. */
+ * page is free, and handle accordingly.  All pages are marked as busy by
+ * default, and we're just determining which of them could be free. */
 static void parse_mboot_region(struct multiboot_mmap_entry *entry, void *data)
 {
        physaddr_t boot_freemem_paddr = (physaddr_t)data;
@@ -85,18 +88,15 @@ static void parse_mboot_region(struct multiboot_mmap_entry *entry, void *data)
                 * that memory may be freed later (like the smp_boot page), we'll treat
                 * it like it is busy/allocated. */
                if (i < EXTPHYSMEM)
-                       goto page_busy;
+                       continue;
                /* Mark as busy pages already allocated in boot_alloc() */
                if (in_bootzone && (i < boot_freemem_paddr))
-                       goto page_busy;
+                       continue;
                /* Need to double check for the kernel, in case it wasn't in the
                 * bootzone.  If it was in the bootzone, we already skipped it. */
                if (pa64_is_in_kernel(i))
-                       goto page_busy;
+                       continue;
                track_free_page(pa64_to_page(i));
-               continue;
-page_busy:
-               page_setref(pa64_to_page(i), 1);
        }
 }
 
@@ -104,17 +104,20 @@ static void check_range(uint64_t start, uint64_t end, int expect)
 {
        int ref;
        if (PGOFF(start))
-               printk("Warning: check_range given an unaligned addr %p\n", start);
+               printk("Warning: check_range given unaligned addr 0x%016llx\n", start);
        for (uint64_t i = start; i < end; i += PGSIZE)  {
                ref = kref_refcnt(&pa64_to_page(i)->pg_kref);
                if (ref != expect) {
-                       printk("Error: physaddr %p refcnt was %d, expected %d\n", i, ref,
-                              expect);
+                       printk("Error: while checking range [0x%016llx, 0x%016llx), "
+                              "physaddr 0x%016llx refcnt was %d, expected %d\n", start,
+                              end, i, ref, expect);
                        panic("");
                }
        }
 }
 
+/* Note this doesn't check all of memory.  There are some chunks of 'memory'
+ * that aren't reported by MB at all, like the VRAM sections at 0xa0000. */
 static void check_mboot_region(struct multiboot_mmap_entry *entry, void *data)
 {
        extern char end[];
@@ -124,8 +127,10 @@ static void check_mboot_region(struct multiboot_mmap_entry *entry, void *data)
        /* Need to deal with 32b wrap-around */
        uint64_t zone_end = MIN(entry->addr + entry->len, (uint64_t)max_paddr);
 
-       if (entry->type != MULTIBOOT_MEMORY_AVAILABLE)
+       if (entry->type != MULTIBOOT_MEMORY_AVAILABLE) {
+               check_range(entry->addr, zone_end, 1);
                return;
+       }
        if (zone_end <= EXTPHYSMEM) {
                check_range(entry->addr, zone_end, 1);
                return;
@@ -149,33 +154,78 @@ static void check_mboot_region(struct multiboot_mmap_entry *entry, void *data)
 }
 
 /* Since we can't parse multiboot mmap entries, we need to just guess at what
- * pages are free and which ones aren't.  We'll just go with:
+ * pages are free and which ones aren't.
+ *
+ * Despite the lack of info from mbi, I know there is a magic hole in physical
+ * memory that we can't use, from the IOAPIC_PBASE on up [0xfec00000,
+ * 0xffffffff] (I'm being pessimistic).  But, that's not pessimistic enough!
+ * Qemu still doesn't like that.   From using 0xe0000000 instead works for mine.
+ * According to http://wiki.osdev.org/Memory_Map_(x86), some systems could
+ * reserve from [0xc0000000, 0xffffffff].  Anyway, in lieu of real memory
+ * detection, I'm just skipping that entire region.
+ *
+ * We may or may not have more free memory above this magic hole, depending on
+ * both the amount of RAM we have as well as 32 vs 64 bit.
+ *
+ * So we'll go with two free memory regions:
  *
  *             [ 0, ROUNDUP(boot_freemem_paddr, PGSIZE) ) = busy
- *             [ ROUNDUP(boot_freemem_paddr, PGSIZE), max_paddr ) = free
+ *             [ ROUNDUP(boot_freemem_paddr, PGSIZE), TOP_OF_1 ) = free
+ *             [ MAGIC_HOLE, 0x0000000100000000 ) = busy
+ *             (and maybe this:)
+ *             [ 0x0000000100000000, max_paddr ) = free
+ *
+ * where TOP_OF_1 is the min of IOAPIC_PBASE and max_paddr.
  *
- * This will ignore the hairy areas below EXTPHYSMEM, and mark the entire kernel
- * and anything we've boot alloc'd as busy. */
+ * For the busy regions, I don't actually need to mark the pages as busy.  They
+ * were marked busy when the pages array was created (same as when we parse
+ * multiboot info).  I'll just assert that they are properly marked as busy.
+ *
+ * As with parsing mbi regions, this will ignore the hairy areas below
+ * EXTPHYSMEM, and mark the entire kernel and anything we've boot alloc'd as
+ * busy. */
 static void account_for_pages(physaddr_t boot_freemem_paddr)
 {
        physaddr_t top_of_busy = ROUNDUP(boot_freemem_paddr, PGSIZE);
+       physaddr_t top_of_free_1 = MIN(0xc0000000, max_paddr);
+       physaddr_t start_of_free_2;
+
+       printk("Warning: poor memory detection (qemu?).  May lose 1GB of RAM\n");
        for (physaddr_t i = 0; i < top_of_busy; i += PGSIZE)
-               page_setref(pa64_to_page(i), 1);
-       for (physaddr_t i = top_of_busy; i < max_paddr; i += PGSIZE)
+               assert(kref_refcnt(&pa64_to_page(i)->pg_kref) == 1);
+       for (physaddr_t i = top_of_busy; i < top_of_free_1; i += PGSIZE)
+               track_free_page(pa64_to_page(i));
+       /* If max_paddr is less than the start of our potential second free mem
+        * region, we can just leave.  We also don't want to poke around the pages
+        * array either (and accidentally run off the end of the array).
+        *
+        * Additionally, 32 bit doesn't acknowledge pmem above the 4GB mark. */
+#ifdef CONFIG_X86_64
+       start_of_free_2 = 0x0000000100000000;
+       if (max_paddr < start_of_free_2)
+               return;
+       for (physaddr_t i = top_of_free_1; i < start_of_free_2; i += PGSIZE)
+               assert(kref_refcnt(&pa64_to_page(i)->pg_kref) == 1);
+       for (physaddr_t i = start_of_free_2; i < max_paddr; i += PGSIZE)
                track_free_page(pa64_to_page(i));
+#endif /* CONFIG_X86_64 */
 }
 
 /* Initialize the memory free lists.  After this, do not use boot_alloc. */
 void page_alloc_init(struct multiboot_info *mbi)
 {
        page_alloc_bootstrap();
-       /* To init the free list(s), each page that is already allocated/busy will
-        * get increfed.  All other pages that were reported as 'free' will be added
-        * to a free list.  Their refcnts are all 0 (when pages was memset).
+       /* First, we need to initialize the pages array such that all memory is busy
+        * by default.
+        *
+        * To init the free list(s), each page that is already allocated/busy will
+        * remain increfed.  All other pages that were reported as 'free' will be
+        * added to a free list.  Their refcnts are set to 0.
         *
         * To avoid a variety of headaches, any memory below 1MB is considered busy.
         * Likewise, everything in the kernel, up to _end is also busy.  And
-        * everything we've already boot_alloc'd is busy.
+        * everything we've already boot_alloc'd is busy.  These chunks of memory
+        * are reported as 'free' by multiboot.
         *
         * We'll also abort the mapping for any addresses over max_paddr, since
         * we'll never use them.  'pages' does not track them either.
@@ -186,8 +236,10 @@ void page_alloc_init(struct multiboot_info *mbi)
         * Finally, if we want to use actual jumbo page allocation (not just
         * mapping), we need to round up _end, and make sure all of multiboot's
         * sections are jumbo-aligned. */
-       physaddr_t boot_freemem_paddr = PADDR(PTRROUNDUP(boot_freemem, PGSIZE));
+       physaddr_t boot_freemem_paddr = PADDR(ROUNDUP(boot_freemem, PGSIZE));
 
+       for (long i = 0; i < max_nr_pages; i++)
+               page_setref(&pages[i], 1);
        if (mboot_has_mmaps(mbi)) {
                mboot_foreach_mmap(mbi, parse_mboot_region, (void*)boot_freemem_paddr);
                /* Test the page alloc - if this gets slow, we can CONFIG it */
index 443ae85..b7d455b 100644 (file)
@@ -18,7 +18,7 @@ typedef unsigned long pde_t;
  *
  *    4 Gig -------->  +------------------------------+
  *                     :              .               :
- *  KERN_VMAP_TOP      +------------------------------+ 0xfffff000
+ *  KERN_VMAP_TOP      +------------------------------+ 0xfec00000
  *                     |                              |
  *                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ RW/--
  *                     :              .               :
@@ -80,6 +80,9 @@ typedef unsigned long pde_t;
  *     mapped.  "Empty Memory" is normally unmapped, but user programs may
  *     map pages there if desired.  ROS user programs map pages temporarily
  *     at UTEMP.
+ *
+ *     KERN_VMAP_TOP is set to the IO_APIC_BASE, where we'll map in the IOAPIC
+ *     and LAPIC.  We need to not give out this region as free pages.
  */
 
 
index 8092dc3..88d5990 100644 (file)
@@ -52,13 +52,21 @@ void mboot_detect_memory(struct multiboot_info *mbi)
        /* mem_lower and upper are measured in KB.  They are 32 bit values, so we're
         * limited to 4TB total. */
        basemem = ROUNDDOWN((size_t)mbi->mem_lower * 1024, PGSIZE);
+       /* On 32 bit, This shift << 10 could cause us to lose some memory, but we
+        * weren't going to access it anyways (won't go beyond ~1GB) */
        extmem = ROUNDDOWN((size_t)mbi->mem_upper * 1024, PGSIZE);
        /* Calculate the maximum physical address based on whether or not there is
         * any extended memory. */
-       if (extmem)
+       if (extmem) {
                max_bios_mem = EXTPHYSMEM + extmem;
-       else
+               /* On 32 bit, if we had enough RAM that adding a little wrapped us
+                * around, we'll back off a little and run with just extmem amount (in
+                * essence, subtracing 1MB). */
+               if (max_bios_mem < extmem)
+                       max_bios_mem = extmem;
+       } else {
                max_bios_mem = basemem;
+       }
        max_bios_addr = MIN(max_bios_mem, KERN_VMAP_TOP - KERNBASE);
        printk("Base memory: %luK, Extended memory: %luK\n", basemem / 1024,
               extmem / 1024);