feat: Bochs VBE linear framebuffer support by GloriousAlpaca · Pull Request #2523 · hermit-os/kernel

GloriousAlpaca · 2026-07-03T15:32:23Z

I'm currently working on porting Doom to Hermit and needed a feature to directly write pixel data into the Bochs Graphics Adaptor as Hermit currently only has support for VGA in textmode.

The feature currently adds standard BGA preparation (Link to OSDev) with a hardcoded resolution (640x400), bpp (32) and a systemcall (sys_get_framebuffer) to receive the address of the framebuffer.
The systemcall falls back to 0 if there was a failure in initializing the framebuffer (e.g. if pci is missing, or the bga feature is not enabled).

I have currently only tested this feature with c programs on a mac using Qemu. The feature is also only available on the x86_64 architecture.

github-actions

Benchmark Results

Details

Benchmark	Current: `0c5a663`	Previous: `274120e`	Performance Ratio
startup_benchmark Build Time	`80.58` s	`84.35` s	`0.96` ❗
startup_benchmark File Size	`0.79` MB	`0.79` MB	`1.00` ❗
Startup Time - 1 core	`0.75` s (`±0.02` s)	`0.77` s (`±0.03` s)	`0.97`
Startup Time - 2 cores	`0.75` s (`±0.02` s)	`0.75` s (`±0.03` s)	`0.99`
Startup Time - 4 cores	`0.75` s (`±0.02` s)	`0.76` s (`±0.02` s)	`1.00`
multithreaded_benchmark Build Time	`82.89` s	`84.42` s	`0.98` ❗
multithreaded_benchmark File Size	`0.90` MB	`0.90` MB	`1.00` ❗
Multithreaded Pi Efficiency - 2 Threads	`92.01` % (`±5.74` %)	`90.82` % (`±6.42` %)	`1.01`
Multithreaded Pi Efficiency - 4 Threads	`44.48` % (`±2.47` %)	`43.89` % (`±3.46` %)	`1.01`
Multithreaded Pi Efficiency - 8 Threads	`25.81` % (`±1.14` %)	`25.48` % (`±1.20` %)	`1.01`
micro_benchmarks Build Time	`82.14` s	`77.88` s	`1.05` ❗
micro_benchmarks File Size	`0.90` MB	`0.90` MB	`1.00` ❗
Scheduling time - 1 thread	`66.25` ticks (`±3.02` ticks)	`63.97` ticks (`±1.84` ticks)	`1.04`
Scheduling time - 2 threads	`36.92` ticks (`±4.95` ticks)	`35.41` ticks (`±4.75` ticks)	`1.04`
Micro - Time for syscall (getpid)	`4.11` ticks (`±0.63` ticks)	`4.34` ticks (`±0.64` ticks)	`0.95`
Memcpy speed - (built_in) block size 4096	`82434.64` MByte/s (`±56999.12` MByte/s)	`86522.47` MByte/s (`±60219.67` MByte/s)	`0.95`
Memcpy speed - (built_in) block size 1048576	`30665.59` MByte/s (`±24746.54` MByte/s)	`31241.67` MByte/s (`±25237.59` MByte/s)	`0.98`
Memcpy speed - (built_in) block size 16777216	`27362.87` MByte/s (`±22702.63` MByte/s)	`30005.84` MByte/s (`±24779.13` MByte/s)	`0.91`
Memset speed - (built_in) block size 4096	`82492.78` MByte/s (`±57042.81` MByte/s)	`86580.61` MByte/s (`±60260.87` MByte/s)	`0.95`
Memset speed - (built_in) block size 1048576	`31428.92` MByte/s (`±25182.48` MByte/s)	`31974.11` MByte/s (`±25664.82` MByte/s)	`0.98`
Memset speed - (built_in) block size 16777216	`28099.76` MByte/s (`±23139.00` MByte/s)	`30781.36` MByte/s (`±25217.87` MByte/s)	`0.91`
Memcpy speed - (rust) block size 4096	`72926.07` MByte/s (`±50972.41` MByte/s)	`77609.40` MByte/s (`±54422.24` MByte/s)	`0.94`
Memcpy speed - (rust) block size 1048576	`30435.32` MByte/s (`±24613.20` MByte/s)	`30933.46` MByte/s (`±25130.96` MByte/s)	`0.98`
Memcpy speed - (rust) block size 16777216	`26696.00` MByte/s (`±22105.83` MByte/s)	`30071.00` MByte/s (`±24839.84` MByte/s)	`0.89`
Memset speed - (rust) block size 4096	`73572.23` MByte/s (`±51416.02` MByte/s)	`78205.56` MByte/s (`±54810.66` MByte/s)	`0.94`
Memset speed - (rust) block size 1048576	`31179.60` MByte/s (`±25036.76` MByte/s)	`31688.70` MByte/s (`±25566.33` MByte/s)	`0.98`
Memset speed - (rust) block size 16777216	`27461.68` MByte/s (`±22581.66` MByte/s)	`30846.72` MByte/s (`±25279.00` MByte/s)	`0.89`
alloc_benchmarks Build Time	`76.15` s	`76.71` s	`0.99` ❗
alloc_benchmarks File Size	`0.87` MB	`0.87` MB	`1`
Allocations - Allocation success	`91.31` %	`91.31` %	`1`
Allocations - Deallocation success	`100.00` %	`100.00` %	`1`
Allocations - Pre-fail Allocations	`61.44` %	`61.44` %	`1`
Allocations - Average Allocation time	`5264.23` Ticks (`±99.04` Ticks)	`5252.14` Ticks (`±125.49` Ticks)	`1.00`
Allocations - Average Allocation time (no fail)	`5995.14` Ticks (`±106.69` Ticks)	`6034.63` Ticks (`±136.23` Ticks)	`0.99`
Allocations - Average Deallocation time	`1430.00` Ticks (`±237.20` Ticks)	`1540.62` Ticks (`±261.22` Ticks)	`0.93`
mutex_benchmark Build Time	`76.77` s	`76.45` s	`1.00` ❗
mutex_benchmark File Size	`0.90` MB	`0.90` MB	`1.00` ❗
Mutex Stress Test Average Time per Iteration - 1 Threads	`12.10` ns (`±0.36` ns)	`12.14` ns (`±0.35` ns)	`1.00`
Mutex Stress Test Average Time per Iteration - 2 Threads	`41.52` ns (`±2.29` ns)	`41.48` ns (`±2.26` ns)	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

mkroening

Thanks for the PR! :)

This is great! First I was wondering why we should use BGA instead of UEFI's GOP, which would also work on real machines, but I guess BGA is just easier at the moment. Since it is not that invasive, this is fine with me.

I am a bit confused regarding naming; maybe you could clear that up in a doc comment of the module or even the feature documentation. If I understand correctly, BGA is the Bochs Graphics Adapter, which we talk to via the VESA BIOS Extensions (VBE) API. But instead of talking to the BIOS via interrupts, we can just talk to the VBE API through the special BGA ports, right? In that case, the module names, feature names, and constant names make sense to me now.

mkroening · 2026-07-07T12:40:24Z

+/// Returns the address of the framebuffer, if available. Returns 0 if no framebuffer is available.
+#[cfg(all(target_arch = "x86_64", feature = "bga"))]
+#[hermit_macro::system]
+#[unsafe(no_mangle)]
+pub extern "C" fn sys_get_framebuffer() -> u64 {
+	crate::kernel::bga::get_framebuffer_address()
+}


The design of system calls needs special scrutiny and discussion.

First, if the user is meant to write into the frame buffer through the return value of this system call, the return type must be a pointer type, not just an integer address.

Second, I am not that deep into graphics, but is the size and dimensions not something that is essential when drawing into a framebuffer? Is that currently just hardcoded? My gut feeling would be to propose something as follows:

pub extern "C" fn sys_framebuffer(framebuffer: *mut Framebuffer) -> c_int; #[repr(C)] pub struct Framebuffer { height: u32, width: u32, size: u64, }

This is just an initial thought, though. It would be useful to look into related existing APIs such as libdrm and UEFI's Graphics Output Protocol (uefi::proto::console::gop::GraphicsOutput).

Another consideration might be multi-monitor support or more properties that a framebuffer has. Of course, the design of this PR does not need to be the last API ever on Hermit. We can replace it if necessary. Still, it would be nice to at least discuss some degree of future-proofing.

Yes this sounds good, I'd maybe setup the struct in a similar way to UEFI Under EFI_GRAPHICS_OUTPUT_MODE_INFORMATION

#[repr(C)] pub struct FramebufferInfo { pub address: *mut u8, pub width: u32, pub height: u32, pub bpp: u32, }

I'd then change the syscall to take a pointer with which to fill the data and return either 0 or an error code (e.g. if no hardware is present):

sys_get_framebuffer(info: *mut FramebufferInfo, struct_size: usize) -> i32

(Struct Size so we can expand the Syscall later, while legacy code still works as expected)

I don't think BGA has multi-monitor support itself, the driver would probably have to look different for this and it would probably make more sense to do virtio-gpu instead. Virtio could then just expand the same struct downwards (for example with pitch or bitmasks).

mkroening · 2026-07-07T12:47:05Z

+use crate::drivers::pci::PciDevice;
+use crate::kernel::pci::PciConfigRegion;
+
+static FRAMEBUFFER_PHYS: AtomicU64 = AtomicU64::new(0);


Could you make this an atomic pointer to preserve provenance?

Suggested change

static FRAMEBUFFER_PHYS: AtomicU64 = AtomicU64::new(0);

static FRAMEBUFFER: AtomicPtr<u8> = AtomicPtr::new(ptr::null_mut());

If we need to store more info such as dimensions and size, a OnceCell might make sense, instead.

mkroening · 2026-07-07T12:49:35Z

+	let bpp: u16 = 32;
+
+	unsafe {
+		let mut index_port: Port<u16> = Port::new(VBE_DISPI_IOPORT_INDEX);


This is not meant to be read, no?

You could make this a write-only port, then.

mkroening · 2026-07-07T12:59:08Z

+const VBE_DISPI_DISABLED: u16 = 0x00;
+const VBE_DISPI_ENABLED: u16 = 0x01;
+const VBE_DISPI_LFB_ENABLED: u16 = 0x40;
+const VBE_DISPI_ID5: u16 = 0xb0c5;


Could you add the missing IDs as well as VBE_DISPI_NOCLEARMEM for completeness?

Do you think the IDs would be a candidate for grouping in an enum?

mkroening · 2026-07-07T12:59:59Z

+const VBE_DISPI_ID5: u16 = 0xb0c5;
+
+pub fn init_device(adapter: &PciDevice<PciConfigRegion>) {
+	//To Do: Detect Resolution automatically


Do you want to do this in this PR or in another, or is this meant for other people in the future?

mkroening · 2026-07-07T13:05:41Z

+		index_port.write(VBE_DISPI_INDEX_ENABLE);
+		data_port.write(VBE_DISPI_DISABLED);


We should have some abstractions here. What about something like this?

struct BgaRegisters; impl BgaRegisters { pub fn read(index: u16) -> u16; pub fn write(index: u16, value: u16); }

mkroening · 2026-07-07T13:08:48Z

+	FRAMEBUFFER_PHYS.store(phys_addr, Ordering::Release);
+
+	assert!(
+		size % 4096 == 0,


I am wondering why clippy::manual_is_multiple_of is not triggering here.

mkroening · 2026-07-07T13:09:28Z

+
+	let mut flags = PageTableEntryFlags::empty();
+	flags.device().writable().execute_disable();
+	flags.insert(PageTableEntryFlags::USER_ACCESSIBLE);


Why do we need this? The application is still running in kernel mode, no?

mkroening · 2026-07-07T13:25:54Z

+## Enables the _Bochs Graphics Adapter_ (BGA) driver.
+##
+## This provides a linear framebuffer for pixel graphics and is primarily useful in QEMU/Bochs environments (x86-64).
+bga = ["pci"]


This should also mention that this does not enable any kernel rendering on the device and that applications must be ported to use this framebuffer.

mkroening · 2026-07-07T13:40:07Z

Ah, please also ensure that your PR does not include merge commits. Please rebase instead.

mkroening · 2026-07-07T13:44:26Z

+	let height: u16 = 400;
+	let bpp: u16 = 32;
+
+	unsafe {


Please try to have as few unsafe operations per block as possible. Ideally, only one.

mkroening self-assigned this Jul 3, 2026

mkroening self-requested a review July 3, 2026 15:40

github-actions Bot reviewed Jul 5, 2026

View reviewed changes

mkroening requested changes Jul 7, 2026

View reviewed changes

mkroening reviewed Jul 7, 2026

View reviewed changes

Andreas Wessing added 2 commits July 7, 2026 16:02

add Bochs VBE linear framebuffer support

a91809b

style: use variable in format string

0c5a663

GloriousAlpaca force-pushed the pr-bga-clean branch from 2792b90 to 0c5a663 Compare July 7, 2026 14:04

	static FRAMEBUFFER_PHYS: AtomicU64 = AtomicU64::new(0);
	static FRAMEBUFFER: AtomicPtr<u8> = AtomicPtr::new(ptr::null_mut());

		index_port.write(VBE_DISPI_INDEX_ENABLE);
		data_port.write(VBE_DISPI_DISABLED);

Uh oh!

Conversation

GloriousAlpaca commented Jul 3, 2026

Uh oh!

github-actions Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Benchmark Results

Uh oh!

mkroening left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GloriousAlpaca Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkroening Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkroening commented Jul 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot left a comment •

edited

Loading

GloriousAlpaca Jul 7, 2026 •

edited

Loading

mkroening Jul 7, 2026 •

edited

Loading