x86: fixes initialization errors in page_alloc
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 1 Aug 2013 23:32:07 +0000 (16:32 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 2 Aug 2013 00:00:16 +0000 (17:00 -0700)
If the kernel's end symbol was page aligned, pa64_is_in_kernel() would
inaccurately report that address as being in the kernel.  That page actually
can be free.  check_range() was catching this.  Luckily, the error was in the
safe direction; we were marking a free page as busy.

The ROUNDDOWN fix is similar.  Check the comment for details; we're mostly
ignoring the issue for now.

kern/arch/x86/page_alloc.c

index 6f8ec8e..eb26a8e 100644 (file)
@@ -50,10 +50,10 @@ static bool pa64_is_in_kernel(uint64_t paddr)
 {
        extern char end[];
        /* kernel is linked and loaded here (in kernel{32,64}.ld */
 {
        extern char end[];
        /* kernel is linked and loaded here (in kernel{32,64}.ld */
-       return (EXTPHYSMEM <= paddr) && (paddr <= PADDR(end));
+       return (EXTPHYSMEM <= paddr) && (paddr < PADDR(end));
 }
 
 }
 
-/* Helper.  For every page in the entry, this will determine whther or not the
+/* Helper.  For every page in the entry, this will determine whether or not the
  * page is free, and handle accordingly. */
 static void parse_mboot_region(struct multiboot_mmap_entry *entry, void *data)
 {
  * page is free, and handle accordingly. */
 static void parse_mboot_region(struct multiboot_mmap_entry *entry, void *data)
 {
@@ -64,7 +64,17 @@ static void parse_mboot_region(struct multiboot_mmap_entry *entry, void *data)
        if (entry->type != MULTIBOOT_MEMORY_AVAILABLE)
                return;
        /* TODO: we'll have some issues with jumbo allocation */
        if (entry->type != MULTIBOOT_MEMORY_AVAILABLE)
                return;
        /* TODO: we'll have some issues with jumbo allocation */
-       for (uint64_t i = ROUNDUP(entry->addr, PGSIZE);
+       /* Most entries are page aligned, though on some machines below EXTPHYSMEM
+        * we may have some that aren't.  If two regions collide on the same page
+        * (one of them starts unaligned), we need to only handle the page once, and
+        * err on the side of being busy.
+        *
+        * Since these regions happen below EXTPHYSMEM, they are all marked busy (or
+        * else we'll panic).  I'll probably rewrite this for jumbos before I find a
+        * machine with unaligned mboot entries in higher memory. */
+       if (PGOFF(entry->addr))
+               assert(entry->addr < EXTPHYSMEM);
+       for (uint64_t i = ROUNDDOWN(entry->addr, PGSIZE);
             i < entry->addr + entry->len;
             i += PGSIZE) {
                /* Skip pages we'll never map (above KERNBASE).  Once we hit one of
             i < entry->addr + entry->len;
             i += PGSIZE) {
                /* Skip pages we'll never map (above KERNBASE).  Once we hit one of