From 1802c5ea2ea2e3f9a641552e4bfafaac35273b69 Mon Sep 17 00:00:00 2001
From: "Justin C. Miller" <justin@devjustinian.com>
Date: Tue, 27 Jul 2021 19:45:34 -0700
Subject: [PATCH] [boot] Seperate video out from console

Separate the video mode setting out from the console code into video.*,
and remove the framebuffer from the kernel args, moving it to the new
init args format.
---
 src/boot/console.cpp            | 107 ++--------------------------
 src/boot/console.h              |  13 ++--
 src/boot/main.cpp               |  19 +++--
 src/boot/module.toml            |   1 +
 src/boot/status.cpp             |  23 +++---
 src/boot/status.h               |  15 ++--
 src/boot/video.cpp              | 121 ++++++++++++++++++++++++++++++++
 src/boot/video.h                |  32 +++++++++
 src/include/init_args.h         |  59 ++++++++++++++++
 src/include/kernel_args.h       |  17 -----
 src/kernel/main.cpp             |  22 +-----
 src/kernel/memory_bootstrap.cpp |  21 ------
 12 files changed, 256 insertions(+), 194 deletions(-)
 create mode 100644 src/boot/video.cpp
 create mode 100644 src/boot/video.h
 create mode 100644 src/include/init_args.h

diff --git a/src/boot/console.cpp b/src/boot/console.cpp
index 4b09605..570d73c 100644
--- a/src/boot/console.cpp
+++ b/src/boot/console.cpp
@@ -1,9 +1,6 @@
 #include <stddef.h>
 #include <stdint.h>
 
-#include <uefi/boot_services.h>
-#include <uefi/graphics.h>
-#include <uefi/protos/graphics_output.h>
 #include <uefi/protos/simple_text_output.h>
 #include <uefi/types.h>
 
@@ -34,22 +31,23 @@ wstrlen(const wchar_t *s)
 }
 
 
-console::console(uefi::boot_services *bs, uefi::protos::simple_text_output *out) :
+console::console(uefi::protos::simple_text_output *out) :
 	m_rows {0},
 	m_cols {0},
-	m_out {out},
-	m_fb {0, 0}
+	m_out {out}
 {
-	pick_mode(bs);
-
 	try_or_raise(
 		m_out->query_mode(m_out->mode->mode, &m_cols, &m_rows),
 		L"Failed to get text output mode.");
-
 	try_or_raise(
 		m_out->clear_screen(),
 		L"Failed to clear screen");
+	s_console = this;
+}
 
+void
+console::announce()
+{
 	m_out->set_attribute(uefi::attribute::light_cyan);
 	m_out->output_string(L"jsix loader ");
 
@@ -58,97 +56,6 @@ console::console(uefi::boot_services *bs, uefi::protos::simple_text_output *out)
 
 	m_out->set_attribute(uefi::attribute::light_gray);
 	m_out->output_string(L" booting...\r\n");
-
-	if (m_fb.type != kernel::init::fb_type::none) {
-		wchar_t const * type = nullptr;
-		switch (m_fb.type) {
-		case kernel::init::fb_type::rgb8:
-			type = L"rgb8";
-			break;
-		case kernel::init::fb_type::bgr8:
-			type = L"bgr8";
-			break;
-		default:
-			type = L"unknown";
-		}
-
-		printf(L"Found framebuffer: %dx%d[%d] type %s @0x%x\r\n",
-			m_fb.horizontal, m_fb.vertical, m_fb.scanline, type, m_fb.phys_addr);
-	} else {
-		printf(L"No framebuffer found.\r\n");
-	}
-
-	s_console = this;
-}
-
-void
-console::pick_mode(uefi::boot_services *bs)
-{
-	uefi::status status;
-	uefi::protos::graphics_output *gfx_out_proto;
-	uefi::guid guid = uefi::protos::graphics_output::guid;
-
-	m_fb.type = kernel::init::fb_type::none;
-
-	uefi::status has_gop = bs->locate_protocol(&guid, nullptr,
-		(void **)&gfx_out_proto);
-
-	if (has_gop != uefi::status::success)
-		// No video output found, skip it
-		return;
-
-	const uint32_t modes = gfx_out_proto->mode->max_mode;
-	uint32_t best = gfx_out_proto->mode->mode;
-
-	uefi::graphics_output_mode_info *info =
-		(uefi::graphics_output_mode_info *)gfx_out_proto->mode;
-
-	uint32_t res = info->horizontal_resolution * info->vertical_resolution;
-	int pixmode = static_cast<int>(info->pixel_format);
-
-	for (uint32_t i = 0; i < modes; ++i) {
-		size_t size = 0;
-
-		try_or_raise(
-			gfx_out_proto->query_mode(i, &size, &info),
-			L"Failed to find a graphics mode the driver claimed to support");
-
-#ifdef MAX_HRES
-		if (info->horizontal_resolution > MAX_HRES) continue;
-#endif
-
-		const uint32_t new_res = info->horizontal_resolution * info->vertical_resolution;
-		int new_pixmode = static_cast<int>(info->pixel_format);
-
-		if (new_pixmode <= pixmode && new_res >= res) {
-			best = i;
-			res = new_res;
-			pixmode = new_pixmode;
-		}
-	}
-
-	try_or_raise(
-		gfx_out_proto->set_mode(best),
-		L"Failed to set graphics mode");
-
-	if (pixmode <= static_cast<int>(uefi::pixel_format::bgr8)) {
-		m_fb.phys_addr = gfx_out_proto->mode->frame_buffer_base;
-		m_fb.size = gfx_out_proto->mode->frame_buffer_size;
-		m_fb.vertical = gfx_out_proto->mode->info->vertical_resolution;
-		m_fb.horizontal = gfx_out_proto->mode->info->horizontal_resolution;
-		m_fb.scanline = gfx_out_proto->mode->info->pixels_per_scanline;
-
-		switch (gfx_out_proto->mode->info->pixel_format) {
-		case uefi::pixel_format::rgb8:
-			m_fb.type = kernel::init::fb_type::rgb8;
-			break;
-		case uefi::pixel_format::bgr8:
-			m_fb.type = kernel::init::fb_type::bgr8;
-			break;
-		default:
-			m_fb.type = kernel::init::fb_type::none;
-		}
-	}
 }
 
 size_t
diff --git a/src/boot/console.h b/src/boot/console.h
index 0565c05..48a6bb5 100644
--- a/src/boot/console.h
+++ b/src/boot/console.h
@@ -1,12 +1,11 @@
+#pragma once
 /// \file console.h
 /// Text output handler
-#pragma once
+
 #include <stdarg.h>
 #include <stddef.h>
-#include "kernel_args.h"
 
 namespace uefi {
-	struct boot_services;
 namespace protos {
 	struct simple_text_output;
 }}
@@ -17,9 +16,9 @@ namespace boot {
 class console
 {
 public:
-	using framebuffer = kernel::init::framebuffer;
+	console(uefi::protos::simple_text_output *out);
 
-	console(uefi::boot_services *bs, uefi::protos::simple_text_output *out);
+	void announce();
 
 	size_t print_hex(uint32_t n) const;
 	size_t print_dec(uint32_t n) const;
@@ -27,20 +26,16 @@ public:
 	size_t print_long_dec(uint64_t n) const;
 	size_t printf(const wchar_t *fmt, ...) const;
 
-	const framebuffer & fb() const { return m_fb; };
-
 	static console & get() { return *s_console; }
 	static size_t print(const wchar_t *fmt, ...);
 
 private:
 	friend class status_line;
 
-	void pick_mode(uefi::boot_services *bs);
 	size_t vprintf(const wchar_t *fmt, va_list args) const;
 
 	size_t m_rows, m_cols;
 	uefi::protos::simple_text_output *m_out;
-	framebuffer m_fb;
 
 	static console *s_console;
 };
diff --git a/src/boot/main.cpp b/src/boot/main.cpp
index 378937c..e448ca2 100644
--- a/src/boot/main.cpp
+++ b/src/boot/main.cpp
@@ -18,6 +18,7 @@
 #include "memory_map.h"
 #include "paging.h"
 #include "status.h"
+#include "video.h"
 
 #include "kernel_args.h"
 
@@ -92,12 +93,12 @@ check_cpu_supported()
 init::args *
 uefi_preboot(uefi::handle image, uefi::system_table *st)
 {
-	status_line status {L"Performing UEFI pre-boot"};
-
 	uefi::boot_services *bs = st->boot_services;
 	uefi::runtime_services *rs = st->runtime_services;
 
-	memory::init_allocator(bs);
+	status_line status {L"Performing UEFI pre-boot"};
+
+	check_cpu_supported();
 	memory::init_pointer_fixup(bs, rs);
 
 	init::args *args = new init::args;
@@ -157,14 +158,18 @@ extern "C" uefi::status
 efi_main(uefi::handle image, uefi::system_table *st)
 {
 	using namespace boot;
-	console con(st->boot_services, st->con_out);
-	check_cpu_supported();
+
+	uefi::boot_services *bs = st->boot_services;
+	console con(st->con_out);
+
+	memory::init_allocator(bs);
+	video::screen *screen = video::pick_mode(bs);
+	con.announce();
 
 	init::args *args = uefi_preboot(image, st);
 	memory::efi_mem_map map = uefi_exit(args, image, st->boot_services);
 
-	args->video = con.fb();
-	status_bar status {con.fb()}; // Switch to fb status display
+	status_bar status {screen}; // Switch to fb status display
 
 	// Map the kernel to the appropriate address
 	init::program &kernel = args->programs[0];
diff --git a/src/boot/module.toml b/src/boot/module.toml
index 2d57779..183623f 100644
--- a/src/boot/module.toml
+++ b/src/boot/module.toml
@@ -16,4 +16,5 @@ sources = [
 	"paging.cpp",
 	"status.cpp",
 	"support.cpp",
+	"video.cpp",
 ]
diff --git a/src/boot/status.cpp b/src/boot/status.cpp
index fc41132..11491a0 100644
--- a/src/boot/status.cpp
+++ b/src/boot/status.cpp
@@ -4,8 +4,9 @@
 
 #include "console.h"
 #include "error.h"
-#include "kernel_args.h"
+#include "init_args.h"
 #include "status.h"
+#include "video.h"
 
 constexpr int num_boxes = 30;
 
@@ -149,15 +150,15 @@ status_line::do_fail(const wchar_t *message, uefi::status status)
 
 
 
-status_bar::status_bar(kernel::init::framebuffer const &fb) :
-	status(fb.size),
+status_bar::status_bar(video::screen *screen) :
+	status(),
 	m_outer(nullptr)
 {
-	m_size = (fb.vertical / num_boxes) - 1;
-	m_top = fb.vertical - m_size;
-	m_horiz = fb.horizontal;
-	m_fb = reinterpret_cast<uint32_t*>(fb.phys_addr);
-	m_type = static_cast<uint16_t>(fb.type);
+	m_size = (screen->mode.vertical / num_boxes) - 1;
+	m_top = screen->mode.vertical - m_size;
+	m_horiz = screen->mode.horizontal;
+	m_fb = reinterpret_cast<uint32_t*>(screen->framebuffer.pointer);
+	m_type = static_cast<uint16_t>(screen->mode.layout);
 	next();
 
 	if (status::s_current_type == status_bar::type)
@@ -197,14 +198,14 @@ status_bar::do_fail(const wchar_t *message, uefi::status status)
 static uint32_t
 make_color(uint8_t r, uint8_t g, uint8_t b, uint16_t type)
 {
-	switch (static_cast<kernel::init::fb_type>(type)) {
-	case kernel::init::fb_type::bgr8:
+	switch (static_cast<video::layout>(type)) {
+		case video::layout::bgr8:
 		return
 			(static_cast<uint32_t>(b) <<  0) |
 			(static_cast<uint32_t>(g) <<  8) |
 			(static_cast<uint32_t>(r) << 16);
 
-	case kernel::init::fb_type::rgb8:
+		case video::layout::rgb8:
 		return
 			(static_cast<uint32_t>(r) <<  0) |
 			(static_cast<uint32_t>(g) <<  8) |
diff --git a/src/boot/status.h b/src/boot/status.h
index 929d4f7..82a4e62 100644
--- a/src/boot/status.h
+++ b/src/boot/status.h
@@ -5,14 +5,16 @@
 #include <stdint.h>
 #include <uefi/types.h>
 
-namespace kernel {
-namespace init {
-	class framebuffer;
-}
+namespace uefi {
+	struct boot_services;
 }
 
 namespace boot {
 
+namespace video {
+	struct screen;
+}
+
 // Abstract base class for status reporters.
 class status
 {
@@ -92,11 +94,8 @@ class status_bar :
 public:
 	constexpr static unsigned type = 2;
 
-	using framebuffer = kernel::init::framebuffer;
-
 	/// Constructor.
-	/// \arg fb  The framebuffer descriptor to draw to
-	status_bar(kernel::init::framebuffer const &fb);
+	status_bar(video::screen *screen);
 	~status_bar();
 
 	virtual void do_warn(const wchar_t *message, uefi::status status) override;
diff --git a/src/boot/video.cpp b/src/boot/video.cpp
new file mode 100644
index 0000000..32de8c0
--- /dev/null
+++ b/src/boot/video.cpp
@@ -0,0 +1,121 @@
+#include <uefi/boot_services.h>
+#include <uefi/graphics.h>
+#include <uefi/protos/graphics_output.h>
+
+#include "console.h"
+#include "error.h"
+#include "init_args.h"
+#include "video.h"
+
+namespace boot {
+namespace video {
+
+using kernel::init::fb_layout;
+using kernel::init::fb_type;
+using kernel::init::module_flags;
+using kernel::init::module_framebuffer;
+using kernel::init::module_type;
+
+static uefi::protos::graphics_output *
+get_gop(uefi::boot_services *bs)
+{
+	uefi::protos::graphics_output *gop = nullptr;
+	uefi::guid guid = uefi::protos::graphics_output::guid;
+
+	uefi::status has_gop = bs->locate_protocol(&guid, nullptr,
+		(void **)&gop);
+
+	if (has_gop != uefi::status::success)
+		return nullptr;
+
+	return gop;
+}
+
+screen *
+pick_mode(uefi::boot_services *bs)
+{
+	uefi::protos::graphics_output *gop = get_gop(bs);
+	if (!gop) {
+		console::print(L"No framebuffer found.\r\n");
+		return nullptr;
+	}
+
+	uefi::graphics_output_mode_info *info = gop->mode->info;
+
+	uint32_t best = gop->mode->mode;
+	uint32_t res = info->horizontal_resolution * info->vertical_resolution;
+	int pixmode = static_cast<int>(info->pixel_format);
+
+	const uint32_t modes = gop->mode->max_mode;
+	for (uint32_t i = 0; i < modes; ++i) {
+		size_t size = 0;
+		uefi::graphics_output_mode_info *new_info = nullptr;
+
+		try_or_raise(
+			gop->query_mode(i, &size, &new_info),
+			L"Failed to find a graphics mode the driver claimed to support");
+
+		const uint32_t new_res = new_info->horizontal_resolution * new_info->vertical_resolution;
+		int new_pixmode = static_cast<int>(new_info->pixel_format);
+
+		if (new_pixmode <= pixmode && new_res >= res) {
+			best = i;
+			res = new_res;
+			pixmode = new_pixmode;
+		}
+	}
+
+	screen *s = new screen;
+	s->mode = {
+		.vertical = gop->mode->info->vertical_resolution,
+		.horizontal = gop->mode->info->horizontal_resolution,
+		.scanline =  gop->mode->info->pixels_per_scanline,
+		.layout = layout::unknown,
+	};
+
+	s->framebuffer = {
+		.pointer = reinterpret_cast<void*>(gop->mode->frame_buffer_base),
+		.count = gop->mode->frame_buffer_size
+	};
+
+	wchar_t const * type = nullptr;
+	switch (info->pixel_format) {
+		case uefi::pixel_format::rgb8:
+			type = L"rgb8";
+			s->mode.layout = layout::rgb8;
+			break;
+
+		case uefi::pixel_format::bgr8:
+			type = L"bgr8";
+			s->mode.layout = layout::bgr8;
+			break;
+
+		default:
+			type = L"unknown";
+	}
+
+	console::print(L"Found framebuffer: %dx%d[%d] type %s @0x%x\r\n",
+		info->horizontal_resolution, info->vertical_resolution,
+		info->pixels_per_scanline, type, gop->mode->frame_buffer_base);
+
+	try_or_raise(
+		gop->set_mode(best),
+		L"Failed to set graphics mode");
+
+	return s;
+}
+
+void
+make_module(screen *s, module_framebuffer *mod)
+{
+	mod->mod_type = module_type::framebuffer;
+	mod->mod_flags = module_flags::none;
+	mod->mod_length = sizeof(module_framebuffer);
+	mod->type = fb_type::uefi; 
+
+	mod->framebuffer = s->framebuffer;
+	mod->mode = s->mode;
+}
+
+} // namespace video
+} // namespace boot
diff --git a/src/boot/video.h b/src/boot/video.h
new file mode 100644
index 0000000..a4503bb
--- /dev/null
+++ b/src/boot/video.h
@@ -0,0 +1,32 @@
+#pragma once
+/// \file video.h
+/// Video mode handling
+
+#include <stdarg.h>
+#include <stddef.h>
+
+#include "init_args.h"
+
+namespace uefi {
+	struct boot_services;
+}
+
+namespace boot {
+namespace video {
+
+using kernel::init::video_mode;
+using layout = kernel::init::fb_layout;
+
+struct screen {
+	buffer framebuffer;
+	video_mode mode;
+};
+
+/// Pick the best video mode and set up the screen
+screen * pick_mode(uefi::boot_services *bs);
+
+/// Make an init arg module from the video mode
+void make_module(screen *s, kernel::init::module_framebuffer *mod);
+
+} // namespace video
+} // namespace boot
diff --git a/src/include/init_args.h b/src/include/init_args.h
new file mode 100644
index 0000000..3aa1443
--- /dev/null
+++ b/src/include/init_args.h
@@ -0,0 +1,59 @@
+#pragma once
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "counted.h"
+
+namespace kernel {
+namespace init {
+
+enum class module_type : uint8_t {
+	program,
+	framebuffer,
+};
+
+enum class module_flags : uint8_t { none = 0 };
+
+struct module
+{
+	module_type mod_type;
+	module_flags mod_flags;
+	uint32_t mod_length;
+};
+
+struct module_program :
+	public module
+{
+	uintptr_t base_address;
+	char filename[];
+};
+
+enum class fb_layout : uint8_t { rgb8, bgr8, unknown = 0xff };
+enum class fb_type : uint8_t { uefi };
+
+struct video_mode
+{
+	uint32_t vertical;
+	uint32_t horizontal;
+	uint32_t scanline;
+	fb_layout layout;
+};
+
+struct module_framebuffer :
+	public module
+{
+	buffer framebuffer;
+	video_mode mode;
+	fb_type type;
+};
+
+struct modules_page
+{
+	uint8_t count;
+	module *modules;
+	modules_page *next;
+};
+
+} // namespace init
+} // namespace kernel
diff --git a/src/include/kernel_args.h b/src/include/kernel_args.h
index ef87695..823ca05 100644
--- a/src/include/kernel_args.h
+++ b/src/include/kernel_args.h
@@ -25,21 +25,6 @@ struct module {
 	mod_type type;
 };
 
-enum class fb_type : uint16_t {
-	none,
-	rgb8,
-	bgr8
-};
-
-struct framebuffer {
-	uintptr_t phys_addr;
-	size_t size;
-	uint32_t vertical;
-	uint32_t horizontal;
-	uint16_t scanline;
-	fb_type type;
-};
-
 enum class section_flags : uint32_t {
 	none    = 0,
 	execute = 1,
@@ -156,8 +141,6 @@ struct args
 
 	void *runtime_services;
 	void *acpi_table;
-
-	framebuffer video;
 }
 __attribute__((aligned(alignof(max_align_t))));
 
diff --git a/src/kernel/main.cpp b/src/kernel/main.cpp
index 3e79745..4676530 100644
--- a/src/kernel/main.cpp
+++ b/src/kernel/main.cpp
@@ -57,9 +57,6 @@ process * load_simple_process(init::program &program);
 
 unsigned start_aps(lapic &apic, const kutil::vector<uint8_t> &ids, void *kpml4);
 
-/// TODO: not this. this is awful.
-init::framebuffer *fb = nullptr;
-
 void
 init_console()
 {
@@ -107,22 +104,6 @@ kernel_main(init::args *args)
 	uint64_t efer = rdmsr(msr::ia32_efer);
 	log::debug(logs::boot, "Control regs: cr0:%lx cr4:%lx efer:%lx", cr0, cr4, efer);
 
-	bool has_video = false;
-	if (args->video.size > 0) {
-		has_video = true;
-		fb = &args->video;
-
-		const init::framebuffer &video = args->video;
-		log::debug(logs::boot, "Framebuffer: %dx%d[%d] type %d @ %llx size %llx",
-			video.horizontal,
-			video.vertical,
-			video.scanline,
-			video.type,
-			video.phys_addr,
-			video.size);
-		logger_clear_immediate();
-	}
-
 	extern IDT &g_bsp_idt;
 	extern TSS &g_bsp_tss;
 	extern GDT &g_bsp_gdt;
@@ -216,9 +197,8 @@ kernel_main(init::args *args)
 	for (unsigned i = 1; i < args->programs.count; ++i)
 		load_simple_process(args->programs[i]);
 
-	if (!has_video)
-		sched->create_kernel_task(logger_task, scheduler::max_priority/2, true);
 
+	sched->create_kernel_task(logger_task, scheduler::max_priority/2, true);
 	sched->start();
 }
 
diff --git a/src/kernel/memory_bootstrap.cpp b/src/kernel/memory_bootstrap.cpp
index 6487359..9016fe9 100644
--- a/src/kernel/memory_bootstrap.cpp
+++ b/src/kernel/memory_bootstrap.cpp
@@ -228,27 +228,6 @@ initialize_main_user_stack()
 	j6_init_value *initv = nullptr;
 	unsigned n = 0;
 
-	extern init::framebuffer *fb;
-	if (fb) {
-		j6_init_framebuffer *fb_desc = push<j6_init_framebuffer>(tcb->rsp3);
-		kutil::memset(fb_desc, 0, sizeof(j6_init_framebuffer));
-
-		fb_desc->addr = fb->phys_addr;
-		fb_desc->size = fb->size;
-		fb_desc->vertical = fb->vertical;
-		fb_desc->horizontal = fb->horizontal;
-		fb_desc->scanline = fb->scanline;
-
-		if (fb->type == kernel::init::fb_type::bgr8)
-			fb_desc->flags |= 1;
-
-		initv = push<j6_init_value>(tcb->rsp3);
-		initv->type = j6_init_desc_framebuffer;
-		initv->data = fb_desc;
-		++n;
-	}
-
-
 	initv = push<j6_init_value>(tcb->rsp3);
 	initv->type = j6_init_handle_other;
 	initv->handle.type = j6_object_type_system;