From e211707e2db22c9e52ed9c065acf43d493d4b3a9 Mon Sep 17 00:00:00 2001 From: Robert Allan James Date: Tue, 11 Aug 2026 09:42:19 -0400 Subject: [PATCH] starkernel: fb_scroll_rows() -- drop unnecessary volatile from the bulk copy/fill Found while investigating item 4.4g's boot stall (moving console_fb_init() earlier caused a >3-minute hang scrolling the fleet-birth transcript). The GOP framebuffer is mapped write-back RAM (vmm.c:350-363), not cache-disabled MMIO with side effects, so there's no correctness reason for the scroll/fill loops to force one un-batchable volatile access per pixel. g_fb.base stays volatile for other call sites; this function now casts to a plain pointer for its bulk copy only. Confirmed this alone does not fix the 4.4g stall -- the kernel builds at -O0 (no optimization flag anywhere in Makefile.starkernel), so nothing here gets vectorized regardless of the qualifier. That's now documented as its own item, FABRIC.md 4.6. Keeping this fix regardless: it's correct on its own terms independent of 4.4g's outcome. Co-Authored-By: Claude Sonnet 5 --- src/starkernel/hal/framebuffer.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/starkernel/hal/framebuffer.c b/src/starkernel/hal/framebuffer.c index 07b6e1d..3c76437 100644 --- a/src/starkernel/hal/framebuffer.c +++ b/src/starkernel/hal/framebuffer.c @@ -255,15 +255,26 @@ void fb_draw_orientation_test(void) * Each character row is (16 × scale) pixels tall. * The vacated rows at the bottom are filled with bg. * - * Uses a word-wide copy loop — no libc memmove dependency. + * Copies through non-volatile pointers (FABRIC.md item 4.4g performance + * fix, 2026-08-11): the GOP framebuffer is mapped write-back, not + * cache-disabled MMIO (vmm.c:350-363 -- "QEMU's VGA emulation is coherent + * and UC- mapping causes #GP"), so it behaves as ordinary RAM and does not + * need per-word volatile access. `g_fb.base` stays `volatile` for + * single-pixel writes elsewhere in this file (draw calls that must not be + * reordered relative to other framebuffer state), but that qualifier forced + * this copy into one un-batchable 4-byte access per element with no + * compiler vectorization -- catastrophic once boot output exceeds one + * screen (~10^10 volatile accesses for an 18,000-line boot transcript + * under TCG). Casting away volatile here for the bulk copy/fill lets the + * compiler batch these loops normally. */ void fb_scroll_rows(uint32_t char_rows, uint32_t bg) { uint32_t pixel_rows_to_scroll; uint32_t src_y, dst_y; - uint32_t remaining_rows; uint32_t x; uint32_t packed_bg; + uint32_t *fb = (uint32_t *)(uintptr_t)g_fb.base; if (!g_fb.ready || char_rows == 0) return; @@ -278,21 +289,19 @@ void fb_scroll_rows(uint32_t char_rows, uint32_t bg) /* Copy pixel rows upward */ for (dst_y = 0; dst_y + pixel_rows_to_scroll < g_fb.height; dst_y++) { src_y = dst_y + pixel_rows_to_scroll; - volatile uint32_t *src = g_fb.base + src_y * g_fb.stride; - volatile uint32_t *dst = g_fb.base + dst_y * g_fb.stride; + uint32_t *src = fb + src_y * g_fb.stride; + uint32_t *dst = fb + dst_y * g_fb.stride; for (x = 0; x < g_fb.width; x++) { dst[x] = src[x]; } } /* Clear the newly exposed rows at the bottom */ - remaining_rows = g_fb.height - (g_fb.height - pixel_rows_to_scroll); for (dst_y = g_fb.height - pixel_rows_to_scroll; dst_y < g_fb.height; dst_y++) { - volatile uint32_t *row = g_fb.base + dst_y * g_fb.stride; + uint32_t *row = fb + dst_y * g_fb.stride; for (x = 0; x < g_fb.width; x++) { row[x] = packed_bg; } } - (void)remaining_rows; /* calculated inline above */ }