|

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

Closed Chris Tusa opened this issue 54 days ago
Chris Tusa opened this issue 54 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 53 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 53 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