From 85ba70d86f69b2535586a2adcd873f85238b8491 Mon Sep 17 00:00:00 2001 From: Hendiadyoin1 Date: Fri, 18 Feb 2022 16:53:17 +0100 Subject: [PATCH] Kernel: Don't override FramebufferDevice's memory regions on mmap This additionally refactors FramebufferDevice::try_to_initialize to not leave the FramebufferDevice in an invalid state on errors. This also unifies the logic between FramebufferDevice::mmap and FramebufferDevice::try_to_initialize. This comes with the drawback of removing the UNMAP_AFTER_INIT attribute from this function, which wasn't honoured by IntelNativeGraphicsAdapter anyway. --- Kernel/Graphics/FramebufferDevice.cpp | 33 +++++++++++++++++---------- Kernel/Graphics/FramebufferDevice.h | 1 + 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/Kernel/Graphics/FramebufferDevice.cpp b/Kernel/Graphics/FramebufferDevice.cpp index c9b16957a0..e278a3cac0 100644 --- a/Kernel/Graphics/FramebufferDevice.cpp +++ b/Kernel/Graphics/FramebufferDevice.cpp @@ -40,11 +40,10 @@ ErrorOr FramebufferDevice::mmap(Process& process, OpenFileDescr if (range.size() != framebuffer_length) return EOVERFLOW; - m_userspace_real_framebuffer_vmobject = TRY(Memory::AnonymousVMObject::try_create_for_physical_range(m_framebuffer_address, framebuffer_length)); - m_real_framebuffer_vmobject = TRY(Memory::AnonymousVMObject::try_create_for_physical_range(m_framebuffer_address, framebuffer_length)); - m_swapped_framebuffer_vmobject = TRY(Memory::AnonymousVMObject::try_create_with_size(framebuffer_length, AllocationStrategy::AllocateNow)); - m_real_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(*m_real_framebuffer_vmobject, framebuffer_length, "Framebuffer", Memory::Region::Access::ReadWrite)); - m_swapped_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(*m_swapped_framebuffer_vmobject, framebuffer_length, "Framebuffer Swap (Blank)", Memory::Region::Access::ReadWrite)); + TRY(try_to_initialize()); + + if (m_userspace_framebuffer_region) + return m_userspace_framebuffer_region; RefPtr chosen_vmobject; if (m_graphical_writes_enabled) { @@ -99,16 +98,26 @@ void FramebufferDevice::activate_writes() m_graphical_writes_enabled = true; } -UNMAP_AFTER_INIT ErrorOr FramebufferDevice::try_to_initialize() +ErrorOr FramebufferDevice::try_to_initialize() { - // FIXME: Would be nice to be able to unify this with mmap above, but this - // function is UNMAP_AFTER_INIT for the time being. + if (m_initialized) + return {}; + auto framebuffer_length = TRY(buffer_length(0)); framebuffer_length = TRY(Memory::page_round_up(framebuffer_length)); - m_real_framebuffer_vmobject = TRY(Memory::AnonymousVMObject::try_create_for_physical_range(m_framebuffer_address, framebuffer_length)); - m_swapped_framebuffer_vmobject = TRY(Memory::AnonymousVMObject::try_create_with_size(framebuffer_length, AllocationStrategy::AllocateNow)); - m_real_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(*m_real_framebuffer_vmobject, framebuffer_length, "Framebuffer", Memory::Region::Access::ReadWrite)); - m_swapped_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(*m_swapped_framebuffer_vmobject, framebuffer_length, "Framebuffer Swap (Blank)", Memory::Region::Access::ReadWrite)); + + auto real_framebuffer_vmobject = TRY(Memory::AnonymousVMObject::try_create_for_physical_range(m_framebuffer_address, framebuffer_length)); + auto swapped_framebuffer_vmobject = TRY(Memory::AnonymousVMObject::try_create_with_size(framebuffer_length, AllocationStrategy::AllocateNow)); + auto real_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(*real_framebuffer_vmobject, framebuffer_length, "Framebuffer", Memory::Region::Access::ReadWrite)); + auto swapped_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(*swapped_framebuffer_vmobject, framebuffer_length, "Framebuffer Swap (Blank)", Memory::Region::Access::ReadWrite)); + + m_real_framebuffer_vmobject = move(real_framebuffer_vmobject); + m_swapped_framebuffer_vmobject = move(swapped_framebuffer_vmobject); + m_real_framebuffer_region = move(real_framebuffer_region); + m_swapped_framebuffer_region = move(swapped_framebuffer_region); + + m_initialized = true; + return {}; } diff --git a/Kernel/Graphics/FramebufferDevice.h b/Kernel/Graphics/FramebufferDevice.h index 207155db76..7387d200bd 100644 --- a/Kernel/Graphics/FramebufferDevice.h +++ b/Kernel/Graphics/FramebufferDevice.h @@ -67,6 +67,7 @@ private: bool m_graphical_writes_enabled { true }; bool m_write_combine { true }; + bool m_initialized { false }; RefPtr m_userspace_real_framebuffer_vmobject; Memory::Region* m_userspace_framebuffer_region { nullptr };