From 46442170942caf2e4e31cf3a6a1499c8af2b3ec6 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 9 Apr 2020 14:31:47 +0200 Subject: [PATCH] Kernel: Remove "non-operational" ACPI parser state If we don't support ACPI, just don't instantiate an ACPI parser. This is way less confusing than having a special parser class whose only purpose is to do nothing. We now search for the RSDP in ACPI::initialize() instead of letting the parser constructor do it. This allows us to defer the decision to create a parser until we're sure we can make a useful one. --- Kernel/ACPI/ACPIDynamicParser.cpp | 6 ------ Kernel/ACPI/ACPIDynamicParser.h | 3 +-- Kernel/ACPI/ACPIParser.cpp | 34 ++----------------------------- Kernel/ACPI/ACPIParser.h | 25 +++++++++++------------ Kernel/ACPI/ACPIStaticParser.cpp | 18 +--------------- Kernel/ACPI/ACPIStaticParser.h | 4 +--- Kernel/ACPI/Initialize.cpp | 28 +++++++++++++++---------- Kernel/ACPI/Initialize.h | 1 + Kernel/PCI/Initializer.cpp | 18 ++-------------- Kernel/Process.cpp | 3 ++- Kernel/Time/HPET.cpp | 4 ++-- Kernel/Time/TimeManagement.cpp | 10 ++++----- Kernel/init.cpp | 3 --- 13 files changed, 46 insertions(+), 111 deletions(-) diff --git a/Kernel/ACPI/ACPIDynamicParser.cpp b/Kernel/ACPI/ACPIDynamicParser.cpp index 19658f56bb..30b5cf94ff 100644 --- a/Kernel/ACPI/ACPIDynamicParser.cpp +++ b/Kernel/ACPI/ACPIDynamicParser.cpp @@ -30,12 +30,6 @@ namespace Kernel { namespace ACPI { -DynamicParser::DynamicParser() - : IRQHandler(9) -{ - klog() << "ACPI: Dynamic Parsing Enabled, Can parse AML"; -} - DynamicParser::DynamicParser(PhysicalAddress rsdp) : IRQHandler(9) , StaticParser(rsdp) diff --git a/Kernel/ACPI/ACPIDynamicParser.h b/Kernel/ACPI/ACPIDynamicParser.h index 97e35d33b6..f7b80f4953 100644 --- a/Kernel/ACPI/ACPIDynamicParser.h +++ b/Kernel/ACPI/ACPIDynamicParser.h @@ -51,8 +51,7 @@ public: virtual const char* purpose() const override { return "ACPI Parser"; } protected: - DynamicParser(); - explicit DynamicParser(PhysicalAddress); + explicit DynamicParser(PhysicalAddress rsdp); private: void build_namespace(); diff --git a/Kernel/ACPI/ACPIParser.cpp b/Kernel/ACPI/ACPIParser.cpp index f72d49176c..3ea46756ba 100644 --- a/Kernel/ACPI/ACPIParser.cpp +++ b/Kernel/ACPI/ACPIParser.cpp @@ -32,10 +32,9 @@ namespace ACPI { static Parser* s_acpi_parser; -Parser& Parser::the() +Parser* Parser::the() { - ASSERT(s_acpi_parser); - return *s_acpi_parser; + return s_acpi_parser; } void Parser::set_the(Parser& parser) @@ -44,31 +43,6 @@ void Parser::set_the(Parser& parser) s_acpi_parser = &parser; } -Parser::Parser(bool usable) -{ - if (usable) { - klog() << "ACPI: Setting up a functional parser"; - } else { - klog() << "ACPI: Limited Initialization. Vital functions are disabled by a request"; - } -} - -PhysicalAddress Parser::find_table(const char*) -{ - klog() << "ACPI: Requested to search for a table, Abort!"; - return {}; -} - -void Parser::try_acpi_reboot() -{ - klog() << "ACPI: Cannot invoke reboot!"; -} - -void Parser::try_acpi_shutdown() -{ - klog() << "ACPI: Cannot invoke shutdown!"; -} - void Parser::enable_aml_interpretation() { klog() << "ACPI: No AML Interpretation Allowed"; @@ -99,9 +73,5 @@ const FADTFlags::x86_Specific_Flags& Parser::x86_specific_flags() const klog() << "ACPI Limited: x86 specific features cannot be obtained"; ASSERT_NOT_REACHED(); } -bool Parser::is_operable() -{ - return false; -} } } diff --git a/Kernel/ACPI/ACPIParser.h b/Kernel/ACPI/ACPIParser.h index 133c29d786..0fb09fd337 100644 --- a/Kernel/ACPI/ACPIParser.h +++ b/Kernel/ACPI/ACPIParser.h @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -38,33 +39,31 @@ namespace ACPI { class Parser { public: - static Parser& the(); + static Parser* the(); template - static void initialize() + static void initialize(PhysicalAddress rsdp) { - set_the(*new ParserType); + set_the(*new ParserType(rsdp)); } - virtual PhysicalAddress find_table(const char* sig); + virtual PhysicalAddress find_table(const char* sig) = 0; - virtual void try_acpi_reboot(); - virtual bool can_reboot() { return false; } - virtual void try_acpi_shutdown(); - virtual bool can_shutdown() { return false; } + virtual void try_acpi_reboot() = 0; + virtual bool can_reboot() = 0; + virtual void try_acpi_shutdown() = 0; + virtual bool can_shutdown() = 0; - virtual const FADTFlags::HardwareFeatures& hardware_features() const; - virtual const FADTFlags::x86_Specific_Flags& x86_specific_flags() const; + virtual const FADTFlags::HardwareFeatures& hardware_features() const = 0; + virtual const FADTFlags::x86_Specific_Flags& x86_specific_flags() const = 0; virtual void enable_aml_interpretation(); virtual void enable_aml_interpretation(File&); virtual void enable_aml_interpretation(u8*, u32); virtual void disable_aml_interpretation(); - virtual bool is_operable(); protected: - explicit Parser(bool usable = false); - bool m_operable; + Parser() {} private: static void set_the(Parser&); diff --git a/Kernel/ACPI/ACPIStaticParser.cpp b/Kernel/ACPI/ACPIStaticParser.cpp index 64828c672e..994df7053f 100644 --- a/Kernel/ACPI/ACPIStaticParser.cpp +++ b/Kernel/ACPI/ACPIStaticParser.cpp @@ -324,26 +324,10 @@ void StaticParser::locate_main_system_description_table() } } -StaticParser::StaticParser() - : Parser(true) - , m_rsdp(StaticParsing::search_rsdp()) -{ - if (!m_rsdp.is_null()) { - klog() << "ACPI: Using RSDP @ " << m_rsdp; - m_operable = true; - locate_static_data(); - } else { - m_operable = false; - klog() << "ACPI: Disabled, due to RSDP being absent"; - } -} - StaticParser::StaticParser(PhysicalAddress rsdp) - : Parser(true) - , m_rsdp(rsdp) + : m_rsdp(rsdp) { klog() << "ACPI: Using RSDP @ " << rsdp; - m_operable = true; locate_static_data(); } diff --git a/Kernel/ACPI/ACPIStaticParser.h b/Kernel/ACPI/ACPIStaticParser.h index 613ba6a8d6..905995b16e 100644 --- a/Kernel/ACPI/ACPIStaticParser.h +++ b/Kernel/ACPI/ACPIStaticParser.h @@ -41,14 +41,12 @@ public: virtual bool can_reboot() override; virtual bool can_shutdown() override { return false; } virtual void try_acpi_shutdown() override; - virtual bool is_operable() override { return m_operable; } virtual const FADTFlags::HardwareFeatures& hardware_features() const override; virtual const FADTFlags::x86_Specific_Flags& x86_specific_flags() const override; protected: - StaticParser(); - explicit StaticParser(PhysicalAddress); + explicit StaticParser(PhysicalAddress rsdp); private: void locate_static_data(); diff --git a/Kernel/ACPI/Initialize.cpp b/Kernel/ACPI/Initialize.cpp index 4b7f68f7ce..a64ebea97b 100644 --- a/Kernel/ACPI/Initialize.cpp +++ b/Kernel/ACPI/Initialize.cpp @@ -49,17 +49,23 @@ static FeatureLevel determine_feature_level() void initialize() { - switch (determine_feature_level()) { - case FeatureLevel::Enabled: - Parser::initialize(); - break; - case FeatureLevel::Limited: - Parser::initialize(); - break; - case FeatureLevel::Disabled: - Parser::initialize(); - break; - } + auto feature_level = determine_feature_level(); + if (feature_level == FeatureLevel::Disabled) + return; + + auto rsdp = StaticParsing::search_rsdp(); + if (rsdp.is_null()) + return; + + if (feature_level == FeatureLevel::Enabled) + Parser::initialize(rsdp); + else + Parser::initialize(rsdp); +} + +bool is_enabled() +{ + return Parser::the(); } } diff --git a/Kernel/ACPI/Initialize.h b/Kernel/ACPI/Initialize.h index f7d8b158d9..1ff2f6cbb4 100644 --- a/Kernel/ACPI/Initialize.h +++ b/Kernel/ACPI/Initialize.h @@ -29,6 +29,7 @@ namespace Kernel { namespace ACPI { +bool is_enabled(); void initialize(); } diff --git a/Kernel/PCI/Initializer.cpp b/Kernel/PCI/Initializer.cpp index e893474a47..2189a7129f 100644 --- a/Kernel/PCI/Initializer.cpp +++ b/Kernel/PCI/Initializer.cpp @@ -36,13 +36,11 @@ namespace Kernel { namespace PCI { -static bool test_acpi(); static bool test_pci_io(); -static bool test_pci_mmio(); static Access::Type detect_optimal_access_type(bool mmio_allowed) { - if (mmio_allowed && test_acpi() && test_pci_mmio()) + if (mmio_allowed && ACPI::is_enabled() && !ACPI::Parser::the()->find_table("MCFG").is_null()) return Access::Type::MMIO; if (test_pci_io()) @@ -57,7 +55,7 @@ void initialize() bool mmio_allowed = kernel_command_line().lookup("pci_mmio").value_or("off") == "on"; if (detect_optimal_access_type(mmio_allowed) == Access::Type::MMIO) - MMIOAccess::initialize(ACPI::Parser::the().find_table("MCFG")); + MMIOAccess::initialize(ACPI::Parser::the()->find_table("MCFG")); else IOAccess::initialize(); @@ -68,13 +66,6 @@ void initialize() }); } -bool test_acpi() -{ - if ((kernel_command_line().contains("noacpi")) || !ACPI::Parser::the().is_operable()) - return false; - return true; -} - bool test_pci_io() { klog() << "Testing PCI via manual probing... "; @@ -90,10 +81,5 @@ bool test_pci_io() return false; } -bool test_pci_mmio() -{ - return !ACPI::Parser::the().find_table("MCFG").is_null(); -} - } } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 337a27a3c5..e3dae71ba5 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -3993,7 +3993,8 @@ int Process::sys$reboot() dbg() << "syncing mounted filesystems..."; FS::sync(); dbg() << "attempting reboot via ACPI"; - ACPI::Parser::the().try_acpi_reboot(); + if (ACPI::is_enabled()) + ACPI::Parser::the()->try_acpi_reboot(); dbg() << "attempting reboot via KB Controller..."; IO::out8(0x64, 0xFE); diff --git a/Kernel/Time/HPET.cpp b/Kernel/Time/HPET.cpp index 8ada8c40fc..6ad91358ce 100644 --- a/Kernel/Time/HPET.cpp +++ b/Kernel/Time/HPET.cpp @@ -118,7 +118,7 @@ bool HPET::test_and_initialize() { ASSERT(!HPET::initialized()); hpet_initialized = true; - auto hpet = ACPI::Parser::the().find_table("HPET"); + auto hpet = ACPI::Parser::the()->find_table("HPET"); if (hpet.is_null()) return false; klog() << "HPET @ " << hpet; @@ -141,7 +141,7 @@ bool HPET::test_and_initialize() bool HPET::check_for_exisiting_periodic_timers() { - auto hpet = ACPI::Parser::the().find_table("HPET"); + auto hpet = ACPI::Parser::the()->find_table("HPET"); if (hpet.is_null()) return false; auto region = MM.allocate_kernel_region(hpet.page_base(), (PAGE_SIZE * 2), "HPET Initialization", Region::Access::Read); diff --git a/Kernel/Time/TimeManagement.cpp b/Kernel/Time/TimeManagement.cpp index 1a85078216..57cb4b4e5b 100644 --- a/Kernel/Time/TimeManagement.cpp +++ b/Kernel/Time/TimeManagement.cpp @@ -92,8 +92,8 @@ void TimeManagement::stale_function(const RegisterState&) TimeManagement::TimeManagement(bool probe_non_legacy_hardware_timers) { - if (ACPI::Parser::the().is_operable()) { - if (!ACPI::Parser::the().x86_specific_flags().cmos_rtc_not_present) { + if (ACPI::is_enabled()) { + if (!ACPI::Parser::the()->x86_specific_flags().cmos_rtc_not_present) { RTC::initialize(); m_epoch_time += boot_time(); } else { @@ -161,7 +161,7 @@ bool TimeManagement::is_hpet_periodic_mode_allowed() bool TimeManagement::probe_and_set_non_legacy_hardware_timers() { - if (!ACPI::Parser::the().is_operable()) + if (!ACPI::is_enabled()) return false; if (!HPET::test_and_initialize()) return false; @@ -211,8 +211,8 @@ bool TimeManagement::probe_and_set_non_legacy_hardware_timers() bool TimeManagement::probe_and_set_legacy_hardware_timers() { - if (ACPI::Parser::the().is_operable()) { - if (ACPI::Parser::the().x86_specific_flags().cmos_rtc_not_present) { + if (ACPI::is_enabled()) { + if (ACPI::Parser::the()->x86_specific_flags().cmos_rtc_not_present) { dbg() << "ACPI: CMOS RTC Not Present"; return false; } else { diff --git a/Kernel/init.cpp b/Kernel/init.cpp index 824eed5213..ac9fd0a156 100644 --- a/Kernel/init.cpp +++ b/Kernel/init.cpp @@ -167,9 +167,6 @@ void init_stage2() SyncTask::spawn(); FinalizerTask::spawn(); - // Sample test to see if the ACPI parser is working... - klog() << "ACPI: HPET table @ " << ACPI::Parser::the().find_table("HPET"); - PCI::initialize(); if (kernel_command_line().contains("text_debug")) {