Free every usable memory region, not one contiguous run

e820_init() took the usable run *containing the kernel* and treated its
end as the top of RAM, and kinit1() walked a single range from the end
of the kernel image to there. That is the shape of a BIOS map. It is
not the shape of a UEFI one: OVMF places an ACPI NVS region at 8 MB,
so the kernel's run ended there, the map failed the PHYSTOP_MIN check,
and the kernel fell back to asserting 224 MB it had not measured --
then freed straight through the reserved hole anyway.

Keep the usable regions as a list as well as a top address. kinit1()
now frees each one, clipped to the window main() passes in, so the
holes are skipped rather than either freed or fatal. PHYSTOP still
means what it always did -- the top of what kalloc may hand out, and
the bound kfree() rejects pages above -- so no call site changes.

phystop comes from the largest region rather than the highest. The DMA
arena is carved off the top of PHYSTOP and must be physically
contiguous, and the largest region is the only one certain to have
room; a smaller region above it is left unfreed rather than raising
the bound past a hole, because kfree()'s check is worth more than the
pages it costs. Report the usable total separately from the top
address, since with a fragmented map they are different numbers and
printing one as the other is how 8 MB went unnoticed.

Verified on both paths. BIOS: unchanged at 1023 MB in 2 regions,
bootcheck 27/27. UEFI under OVMF: 1018 MB in 7 regions, up from the
8 MB that triggered the fallback, booting through vga, e1000, the RTC
and on to cpu0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

include/memlayout.h

@@ -17,6 +17,28 @@ extern uint64 phystop;
 #define PHYSTOP phystop
 #endif
 
+// PHYSTOP alone describes memory as one run ending at a top address, which is
+// what a BIOS map usually looks like and what UEFI never does: OVMF drops an
+// ACPI NVS region at 0x800000, so the run containing the kernel ends at 8 MB
+// and everything above the hole was invisible. The usable regions are
+// therefore kept as a list as well, and kinit1() frees each one rather than
+// walking a single range from the kernel's end to the top.
+//
+// PHYSTOP still means what it always did -- the top of what kalloc may hand
+// out, and the bound kfree() rejects pages above -- so every existing call
+// site is unaffected.
+#define MEMREGION_MAX 32
+
+#ifndef __ASSEMBLER__
+struct memregion {
+  uint64 base;   // first byte, page aligned
+  uint64 end;    // one past the last byte, page aligned
+};
+
+extern struct memregion mem_usable[MEMREGION_MAX];
+extern int nmem_usable;
+#endif
+
 // What to assume when the firmware gives us nothing usable: the old constant,
 // so that path behaves exactly as the kernel always did.
 #define PHYSTOP_FALLBACK 0xE000000ULL

kernel/arch/x86_64/e820.c

@@ -16,6 +16,7 @@
 #include "defs.h"
 #include "memlayout.h"
 #include "mmu.h"
+#include "dma.h"   // DMA_ARENA_BYTES: the top of PHYSTOP must be able to hold it
 
 // Set before kinit1() and never again. Starts at the old constant so that a
 // crash between entry and e820_init() behaves the way it always did rather
@@ -28,7 +29,25 @@ uint64 phystop = PHYSTOP_FALLBACK;
 #define E820_MAP_ADDR   (0x30000 + 0x208)
 
 #define E820_USABLE     1
-#define E820_MAXENTRIES 32
+#define E820_MAXENTRIES MEMREGION_MAX
+
+// Every usable region, page aligned inwards and clipped to PHYSTOP_MAX.
+// kinit1() frees these; see include/memlayout.h for why a list and not just a
+// top address.
+struct memregion mem_usable[MEMREGION_MAX];
+int nmem_usable;
+
+// The fallback path must still hand kalloc something, or the kernel comes up
+// with an empty free list and dies somewhere far less obvious than here. One
+// region spanning what the old constant asserted reproduces the previous
+// behaviour exactly.
+static void
+assume_fallback(void)
+{
+  mem_usable[0].base = EXTMEM;
+  mem_usable[0].end  = PHYSTOP_FALLBACK;
+  nmem_usable = 1;
+}
 
 struct e820entry {
   uint64 base;
@@ -49,45 +68,68 @@ e820_init(void)
   if (count == 0 || count > E820_MAXENTRIES) {
     cprintf("e820: no usable map (count %d), assuming %d MB\n",
             count, (int)(PHYSTOP_FALLBACK / (1024 * 1024)));
+    assume_fallback();
     return;
   }
 
-  // The end of the usable region that *contains the kernel*, not the highest
-  // usable address anywhere.
+  // Collect every usable region rather than just the one containing the
+  // kernel.
   //
-  // This distinction matters and is easy to get wrong. A memory map is not one
-  // run of RAM: there are reserved holes in it -- ACPI tables, firmware
-  // scratch, the legacy hole below 1 MB -- and kalloc's freerange() walks
-  // straight from the end of the kernel image to phystop handing out every page
-  // between. Taking the global maximum would step over a reserved region and
-  // put firmware tables on the free list, which fails later, elsewhere, and
-  // looks like anything but a memory map problem.
+  // A memory map is not one run of RAM: there are reserved holes in it -- ACPI
+  // tables, firmware scratch, the legacy hole below 1 MB. The old code took the
+  // run containing the kernel and stopped, because kalloc walked a single range
+  // and taking the global maximum would have stepped over a reserved region and
+  // put firmware tables on the free list. That was the right answer for the
+  // wrong shape: under OVMF the kernel's run ends at the ACPI NVS region at
+  // 8 MB, and every byte above it was lost. kalloc now frees region by region,
+  // so the holes are skipped instead of being either freed or fatal.
+  uint64 largest = 0;
   uint64 top = 0;
+  nmem_usable = 0;
   for (uint32 i = 0; i < count; i++) {
     if (e[i].type != E820_USABLE)
       continue;
-    uint64 base = e[i].base, end = base + e[i].length;
-    if (base <= EXTMEM && EXTMEM < end) {
+
+    // Align inwards: a region is only usable where a whole page fits.
+    uint64 base = (e[i].base + PGSIZE - 1) & ~(uint64)(PGSIZE - 1);
+    uint64 end  = (e[i].base + e[i].length) & ~(uint64)(PGSIZE - 1);
+
+    // Above PHYSTOP_MAX the direct map is device memory, not RAM (see below).
+    if (end > PHYSTOP_MAX)
+      end = PHYSTOP_MAX;
+    if (base >= end)
+      continue;
+
+    mem_usable[nmem_usable].base = base;
+    mem_usable[nmem_usable].end  = end;
+    nmem_usable++;
+
+    // phystop comes from the largest region, not the highest. It has to be a
+    // region that can hold the DMA arena, which is carved off the top of
+    // PHYSTOP and must be physically contiguous; the largest is the only one
+    // that is certain to have room. A small usable region above it is left
+    // unfreed rather than raising the bound past a hole -- kfree() rejects
+    // anything at or above PHYSTOP, and that check is worth more than the
+    // stray page it costs.
+    if (end - base > largest) {
+      largest = end - base;
       top = end;
-      break;
     }
   }
 
-  if (top == 0) {
-    cprintf("e820: no usable region covers 0x%x, assuming %d MB\n",
-            EXTMEM, (int)(PHYSTOP_FALLBACK / (1024 * 1024)));
+  if (nmem_usable == 0) {
+    cprintf("e820: no usable regions in %d entries, assuming %d MB\n",
+            count, (int)(PHYSTOP_FALLBACK / (1024 * 1024)));
+    assume_fallback();
     return;
   }
 
-  // kvmalloc() maps physical 0-4 GB with four 1 GB pages, and marks the top one
-  // uncached because that is where PCI BARs and the framebuffer live. So 3 GB
-  // is the ceiling for anything the kernel will treat as RAM: above it the
-  // direct map either does not exist or is device memory that would be
+  // The PHYSTOP_MAX ceiling is applied per region above, when each one is
+  // clipped. kvmalloc() maps physical 0-4 GB with four 1 GB pages and marks the
+  // top one uncached because that is where PCI BARs and the framebuffer live,
+  // so 3 GB is the ceiling for anything the kernel will treat as RAM: above it
+  // the direct map either does not exist or is device memory that would be
   // catastrophically slow to allocate from and is not ours to hand out.
-  if (top > PHYSTOP_MAX)
-    top = PHYSTOP_MAX;
-
-  top &= ~(uint64)(PGSIZE - 1);
 
   // Below this the kernel image, its page tables and the DMA arena do not fit,
   // and continuing produces a failure much harder to read than this line.
@@ -96,10 +138,33 @@ e820_init(void)
             (int)(top / (1024 * 1024)), (int)(PHYSTOP_MIN / (1024 * 1024)));
     // Left at the fallback deliberately rather than lowered: a machine this
     // small is going to fail, and it should fail having said why.
+    assume_fallback();
+    return;
+  }
+
+  // dma_init() carves [PHYSTOP - DMA_ARENA_BYTES, PHYSTOP) and needs it
+  // physically contiguous, so the region that set `top` must be able to hold
+  // it. Picking the largest region makes this true on any machine worth
+  // booting; checking says so out loud if it ever is not.
+  if (largest < DMA_ARENA_BYTES) {
+    cprintf("e820: largest usable region is %d MB, too small for the %d MB "
+            "DMA arena\n", (int)(largest / (1024 * 1024)),
+            (int)(DMA_ARENA_BYTES / (1024 * 1024)));
+    assume_fallback();
     return;
   }
 
   phystop = top;
-  cprintf("e820: %d MB usable (%d entries)\n",
+
+  // Report what is actually usable, not phystop: with a fragmented map those
+  // are different numbers, and printing the top address as though it were the
+  // amount of RAM is how the 8 MB under OVMF went unnoticed for so long.
+  uint64 usable = 0;
+  for (int i = 0; i < nmem_usable; i++)
+    usable += mem_usable[i].end - mem_usable[i].base;
+
+  cprintf("e820: %d MB usable in %d region%s, top %d MB (%d entries)\n",
+          (int)(usable / (1024 * 1024)), nmem_usable,
+          nmem_usable == 1 ? "" : "s",
           (int)(phystop / (1024 * 1024)), count);
 }

kernel/core/kalloc.c

@@ -37,13 +37,47 @@ check_run(struct run *r, char *where)
   }
 }
 
+// vstart..vend is the window the kernel is willing to give away: from the end
+// of its own image to the bottom of the DMA arena. Inside that window the
+// firmware's map decides what is real -- free each usable region clipped to the
+// window, rather than walking the whole span and handing out the holes.
+//
+// The old code walked it as one range, which is what a BIOS map looks like.
+// Under UEFI it is not: OVMF puts an ACPI NVS region at 8 MB, so the run
+// containing the kernel stopped there and everything above it was lost. The
+// window still bounds things at both ends, so nothing below the kernel image
+// and nothing inside the DMA arena is ever freed, whatever the map says.
 void
 kinit1(void *vstart, void *vend)
 {
   initlock(&kmem.lock, "kmem");
   kmem.use_lock = 0;
   kmem.freelist = 0; // empty
-  freerange(vstart, vend);
+
+  // No map at all means e820_init() could not parse one; it leaves a single
+  // synthetic region behind for exactly this reason, so this is belt and
+  // braces rather than a path anything takes.
+  if(nmem_usable == 0){
+    freerange(vstart, vend);
+    return;
+  }
+
+  addr_t lo_limit = V2P((addr_t)vstart);
+  addr_t hi_limit = V2P((addr_t)vend);
+
+  for(int i = 0; i < nmem_usable; i++){
+    addr_t lo = mem_usable[i].base;
+    addr_t hi = mem_usable[i].end;
+
+    if(lo < lo_limit)
+      lo = lo_limit;
+    if(hi > hi_limit)
+      hi = hi_limit;
+    if(lo >= hi)
+      continue;
+
+    freerange(P2V(lo), P2V(hi));
+  }
 }
 
 void