From 4c506f91fecd0d795dd69c49d2d5edbada5ae130 Mon Sep 17 00:00:00 2001 From: Tom Date: Sat, 29 Jan 2022 14:08:05 -0700 Subject: [PATCH] Kernel: Disable BootFramebufferConsole when drivers create a new one When GraphicsManagement initializes the drivers we can disable the bootloader framebuffer console. Right now we don't yet fully destroy the no longer needed console as it may be in use by another CPU. --- Kernel/Graphics/Bochs/GraphicsAdapter.cpp | 3 +-- Kernel/Graphics/GraphicsManagement.cpp | 27 +++++++++++++++++++ Kernel/Graphics/GraphicsManagement.h | 8 +----- .../Graphics/Intel/NativeGraphicsAdapter.cpp | 3 +-- Kernel/Graphics/VGACompatibleAdapter.cpp | 6 ++--- Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp | 3 +-- 6 files changed, 33 insertions(+), 17 deletions(-) diff --git a/Kernel/Graphics/Bochs/GraphicsAdapter.cpp b/Kernel/Graphics/Bochs/GraphicsAdapter.cpp index 166806d7cb..1f4314254e 100644 --- a/Kernel/Graphics/Bochs/GraphicsAdapter.cpp +++ b/Kernel/Graphics/Bochs/GraphicsAdapter.cpp @@ -107,8 +107,7 @@ UNMAP_AFTER_INIT BochsGraphicsAdapter::BochsGraphicsAdapter(PCI::DeviceIdentifie { // We assume safe resolution is 1024x768x32 m_framebuffer_console = Graphics::ContiguousFramebufferConsole::initialize(PhysicalAddress(PCI::get_BAR0(pci_device_identifier.address()) & 0xfffffff0), 1024, 768, 1024 * sizeof(u32)); - // FIXME: This is a very wrong way to do this... - GraphicsManagement::the().m_console = m_framebuffer_console; + GraphicsManagement::the().set_console(*m_framebuffer_console); // Note: If we use VirtualBox graphics adapter (which is based on Bochs one), we need to use IO ports if (pci_device_identifier.hardware_id().vendor_id == 0x80ee && pci_device_identifier.hardware_id().device_id == 0xbeef) diff --git a/Kernel/Graphics/GraphicsManagement.cpp b/Kernel/Graphics/GraphicsManagement.cpp index 7e5d9a773e..aa26927094 100644 --- a/Kernel/Graphics/GraphicsManagement.cpp +++ b/Kernel/Graphics/GraphicsManagement.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -22,6 +23,8 @@ namespace Kernel { static Singleton s_the; +extern Atomic boot_framebuffer_console; + GraphicsManagement& GraphicsManagement::the() { return *s_the; @@ -215,6 +218,15 @@ UNMAP_AFTER_INIT bool GraphicsManagement::initialize() determine_and_initialize_graphics_device(device_identifier); }); + if (!m_console) { + // If no graphics driver was instantiated and we had a bootloader provided + // framebuffer console we can simply re-use it. + if (auto* boot_console = boot_framebuffer_console.load()) { + m_console = *boot_console; + boot_console->unref(); // Drop the leaked reference from Kernel::init() + } + } + if (m_graphics_devices.is_empty()) { dbgln("No graphics adapter was initialized."); return false; @@ -230,4 +242,19 @@ bool GraphicsManagement::framebuffer_devices_exist() const } return false; } + +void GraphicsManagement::set_console(Graphics::Console& console) +{ + m_console = console; + + if (auto* boot_console = boot_framebuffer_console.exchange(nullptr)) { + // Disable the initial boot framebuffer console permanently + boot_console->disable(); + // TODO: Even though we swapped the pointer and disabled the console + // we technically can't safely destroy it as other CPUs might still + // try to use it. Once we solve this problem we can drop the reference + // that we intentionally leaked in Kernel::init(). + } +} + } diff --git a/Kernel/Graphics/GraphicsManagement.h b/Kernel/Graphics/GraphicsManagement.h index 43f964c0f5..6ea0ee5fe5 100644 --- a/Kernel/Graphics/GraphicsManagement.h +++ b/Kernel/Graphics/GraphicsManagement.h @@ -19,14 +19,7 @@ namespace Kernel { -class BochsGraphicsAdapter; -class IntelNativeGraphicsAdapter; -class VGACompatibleAdapter; class GraphicsManagement { - friend class BochsGraphicsAdapter; - friend class IntelNativeGraphicsAdapter; - friend class VGACompatibleAdapter; - friend class Graphics::VirtIOGPU::GraphicsAdapter; public: static GraphicsManagement& the(); @@ -42,6 +35,7 @@ public: Spinlock& main_vga_lock() { return m_main_vga_lock; } RefPtr console() const { return m_console; } + void set_console(Graphics::Console&); void deactivate_graphical_mode(); void activate_graphical_mode(); diff --git a/Kernel/Graphics/Intel/NativeGraphicsAdapter.cpp b/Kernel/Graphics/Intel/NativeGraphicsAdapter.cpp index ba803226b2..7b76ca6696 100644 --- a/Kernel/Graphics/Intel/NativeGraphicsAdapter.cpp +++ b/Kernel/Graphics/Intel/NativeGraphicsAdapter.cpp @@ -209,8 +209,7 @@ IntelNativeGraphicsAdapter::IntelNativeGraphicsAdapter(PCI::Address address) VERIFY(m_framebuffer_height != 0); VERIFY(m_framebuffer_width != 0); m_framebuffer_console = Graphics::ContiguousFramebufferConsole::initialize(framebuffer_address, m_framebuffer_width, m_framebuffer_height, m_framebuffer_pitch); - // FIXME: This is a very wrong way to do this... - GraphicsManagement::the().m_console = m_framebuffer_console; + GraphicsManagement::the().set_console(*m_framebuffer_console); } void IntelNativeGraphicsAdapter::enable_vga_plane() diff --git a/Kernel/Graphics/VGACompatibleAdapter.cpp b/Kernel/Graphics/VGACompatibleAdapter.cpp index 5a62e6e485..4dd899725f 100644 --- a/Kernel/Graphics/VGACompatibleAdapter.cpp +++ b/Kernel/Graphics/VGACompatibleAdapter.cpp @@ -40,8 +40,7 @@ UNMAP_AFTER_INIT VGACompatibleAdapter::VGACompatibleAdapter(PCI::Address address : PCI::Device(address) { m_framebuffer_console = Graphics::TextModeConsole::initialize(*this); - // FIXME: This is a very wrong way to do this... - GraphicsManagement::the().m_console = m_framebuffer_console; + GraphicsManagement::the().set_console(*m_framebuffer_console); } UNMAP_AFTER_INIT VGACompatibleAdapter::VGACompatibleAdapter(PCI::Address address, PhysicalAddress framebuffer_address, size_t framebuffer_width, size_t framebuffer_height, size_t framebuffer_pitch) @@ -52,8 +51,7 @@ UNMAP_AFTER_INIT VGACompatibleAdapter::VGACompatibleAdapter(PCI::Address address , m_framebuffer_pitch(framebuffer_pitch) { m_framebuffer_console = Graphics::ContiguousFramebufferConsole::initialize(framebuffer_address, framebuffer_width, framebuffer_height, framebuffer_pitch); - // FIXME: This is a very wrong way to do this... - GraphicsManagement::the().m_console = m_framebuffer_console; + GraphicsManagement::the().set_console(*m_framebuffer_console); } void VGACompatibleAdapter::enable_consoles() diff --git a/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp b/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp index 49169e3327..df95d3585d 100644 --- a/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp +++ b/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp @@ -43,8 +43,7 @@ void GraphicsAdapter::initialize_framebuffer_devices() create_framebuffer_devices(); m_created_framebuffer_devices = true; - // FIXME: This is a very wrong way to do this... - GraphicsManagement::the().m_console = default_console(); + GraphicsManagement::the().set_console(*default_console()); } void GraphicsAdapter::enable_consoles()