|

BOP-scratch claim runs after kmem_init(), leaving an exposure window #6

Closed Chris Tusa opened this issue 8 days ago
Chris Tusa opened this issue 8 days ago

The BOP-scratch claim runs after kmem_init(), leaving a window where kmem can draw from unclaimed boot pages.

The gap

Three PA spans are claimed off the page free list during startup. Two are
claimed at the right time; one is not:

span claimed in relative to kmem_init()
adapter page tables (bi_adapter_pa) startup_memlist, after kphysm_init() before
BOOTLOADER_RECLAIMABLE (bi_reclaim) startup_memlist, after kphysm_init() before
BOP scratch + its page tables startup_vm, after bop_no_more_mem() after

startup() order is startup_memlist -> startup_kmem -> startup_vm, so
between kphysm_init() and the BOP claim, ~57 MiB of mapped, live boot scratch
sits on the free list with kmem running.

Why it is not simply moved earlier

The claim needs the watermark to be final, and [bi_next_paddr, next_phys) is
only complete after bop_no_more_mem(). Claiming earlier would under-cover, and
under-covering is what rev 328 did — it recovered 1206 pages of ~13000 and left
the rest mapped-and-free.

Severity

Narrow but real, and it is the same class as rev 284 (mapped memory on the free
list, handed out under load). The observed held= count in the claim print is a
partial check: it reports how many pages were already not-free, i.e. how many
something else took during the window. It has been 0 in every run measured so
far, which is evidence the window is not being hit, not evidence it cannot be.

Suggested approach

Either publish a running watermark that is valid earlier, or claim in two
passes: an early conservative claim, then a top-up after bop_no_more_mem()
for whatever moved in between. Watch the held= counter as the regression
signal.

Context: docs/design/LIMINE_ARCHITECTURE.md sections 4 and 8.

Chris Tusa commented 7 days ago

Substantially improved in rev 345 (ee3c758de9b9). Leaving this open — the window is much smaller but not closed, and the issue text needs one correction.

Correction to this issue's own text

The observed held= count in the claim print is a partial check ... It has been
0 in every run measured so far, which is evidence the window is not being hit.

That is not what the tree does. Measured on the pre-change build, the single late
claim printed:

hh: bop pfn [100000,132606) claimed=14525 held=174084 nopp=17733

held=174084, not 0. So the window was not merely theoretical — 174 084 pages in
BOP's span were already not-free by the time the claim ran.

What changed

The suggested "claim in two passes" approach, with the passes kept disjoint:

  • startup_memlist(), before kmem_init(): claim [bi_next_paddr, bop_phys_watermark()) and record how far it reached.
  • startup_vm(), after bop_no_more_mem(): top up from where the first pass
    stopped to the final watermark.

Disjointness is the point. Re-walking the early range in the top-up would report
our own pages as held= and destroy the only signal that print carries.

before   bop        [100000,132606)  claimed=14525   held=174084  nopp=17733
after    bop-early  [100000,11ec02)  claimed=108224  held=0       nopp=17730
         bop-topup  [11ec02,132604)  claimed=2       held=80384   nopp=0

108 224 pages are now claimed while provably still free (held=0 in the early
pass), and the already-taken total falls 174 084 → 80 384.

Why it stays open

The residual is real: BOP's watermark advances from 0x11ec02 to 0x132604
between startup_memlist() and startup_vm() — about 314 MiB — and 80 384 of
those pages are not free when the top-up runs. Roughly half the original exposure
remains, in a window this change does not cover.

Closing it would need either a watermark that is final earlier, or a third claim
point inside that window, or an explanation of what legitimately takes those
pages (which may turn out to be benign — that has not been established either
way). Worth noting the counter is now meaningful: with the passes disjoint,
held= in bop-topup counts only pages something else took during the window.

Verified not to regress anything: full build build_ok=y, boot matrix 28/28
(4 vCPU 12/12), 17 daemons / 0 failed every boot, healthy across 1.2 GiB of kmem
churn.

Chris Tusa commented 7 days ago

Fixed in rev 348 (290cc73c1db2), after measuring the residual with rev 347.

What the residual actually was

The rev-345 top-up printed claimed=2 held=80384 and this issue read the 80 384
as "roughly half the original exposure remains". Instrumenting the allocator
(rev 347) showed otherwise. Between the early claim and the top-up, BOP handed out
314 MiB: 80 215 kernel-VA pages, 170 boot page tables, 1 identity-scratch page.
Every page was free when BOP handed it out — no double allocation. Of the
80 384 pages the top-up found already held:

holder pages how
boot_mapin() 80 214 boot_alloc() callers; hashed into kvp at their kernel VA
htable_attach() 170 kernel-half boot page tables; hashed into kvp at a VA-hole offset
anything else 0

So the "residual exposure" was two pages: init_debug_info()'s page (a
direct BOP_ALLOC that never goes through boot_mapin()) and one scratch
page
, both free — mapped, in use, on the free list — for the whole of
startup_kmem(). Small, but exactly the rev-284 class, and a page table or
debug-info page being handed to kmem is not a small failure.

The 314 MiB itself is kmem_init()'s DEBUG transaction and content logs
(kmem_maxavail()/50 each on an 8 GiB guest) arriving through boot_alloc().
Normal for a DEBUG kernel; not a leak.

The fix

Stop walking the span after the fact; claim at the allocator.

  • startup_memlist(), right after kphysm_init(): bulk-claim
    [bi_next_paddr, watermark) — everything BOP took before page_t's existed —
    and set hh_bop_pp_live (set before the snapshot, so there is no instant at
    which a page can be handed out unclaimed).
  • do_bop_phys_alloc() then claims each page as it hands it out
    (hh_bop_alloc_hook()). A page that is not free at hand-out is a double
    allocation and panics; a page with no page_t is counted.
  • startup_vm(), after bop_no_more_mem(): the walk is now a verifier over
    the whole span and must find claimed=0; otherwise it prints an hh: BUG line
    the boot matrix gates on. (Not a panic — a dead page table released by
    htable_release() would look the same from there.)

This also closes a case no previous version covered: when read_bootenvrc()
clears early_allocation (BOP starting below 4 GiB), allocations descend from
high_phys and [bi_next_paddr, next_phys) no longer describes what BOP took.
Every walk-based claim under-covered there; the hook does not care which way
the allocator moves. The verifier still sees only the ascending span. (That path
is unreachable today: the adapter refuses to boot when the largest USABLE
extent is below 4 GiB — a 2 GiB guest stops at limine_xboot: largest usable region is below 4GiB before the kernel prints anything, verified 2026-09-02.
The hook's direction-independence is defensive.)

Accounting is unchanged: page_reclaim() lowers freemem as the consumers'
own claims did, and availrmem is still adjusted by boot_mapin() /
htable_attach(), whose page_numtopp() accepts an already-held page.

Verification (8 GiB guest, 4 vCPU)

hh: bop-early  pfn [100000,11ec02) claimed=108225 held=0      nopp=17729
hh: bop-verify pfn [100000,132604) claimed=0      held=188611 nopp=17729
hh: bop-verify held: mapin=173910 htable=172 othervn=0 novn=14529
hh: bop-hook at-verify ptable  calls=170  pages=170   claimed=170   nopp=0
hh: bop-hook at-verify scratch calls=1    pages=1     claimed=1     nopp=0
hh: bop-hook at-verify kernel  calls=3530 pages=80215 claimed=80215 nopp=0

108 225 + 170 + 1 + 80 215 = 188 611 = held, to the page. 17 daemons / 0
failed, marker-verified kernel, no panic.

Boot matrix on the fix, every boot gated on bop-verify … claimed=0 + no hh: BUG + no panic: 1 vCPU 4/4, 2 vCPU 12/12, 4 vCPU 12/12. The kernel-class page count is deterministic per vCPU count (80 156 / 80 172 / 80 215 at 1 / 2 / 4).

Docs: rev 349 -- docs/design/LIMINE_ARCHITECTURE.md §4, §5.4, §6, §7, §8; docs/ROADMAP.md.

Assignees

No assignees

Labels

No labels

Severity

Medium