From 793d315994875435418961b92f247a0a81452933 Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 12 Mar 2021 18:35:16 +0200 Subject: [PATCH] Kernel: Don't reset AHCI ports during boot unless requested Instead of blindly resetting every AHCI port, let's just reset only the controller by default. The user can still request to reset everything with a new kernel boot argument called ahci_reset_mode which is set by default to "controller", so the code will only invoke an HBA reset. This kernel boot argument can be set to 3 different values: 1. "controller" - reset the HBA and skip resetting AHCI ports 2. "none" - don't reset anything, so we rely on the firmware to initialize the AHCI HBA and ports for us. 3. "complete" - reset the AHCI HBA and ports. --- Kernel/CommandLine.cpp | 13 +++++++++++++ Kernel/CommandLine.h | 7 +++++++ Kernel/Storage/AHCIController.cpp | 11 +++++++---- Kernel/Storage/AHCIPort.cpp | 16 ++++++++++++++++ Kernel/Storage/AHCIPort.h | 4 ++++ Kernel/Storage/AHCIPortHandler.cpp | 12 +++++++++++- 6 files changed, 58 insertions(+), 5 deletions(-) diff --git a/Kernel/CommandLine.cpp b/Kernel/CommandLine.cpp index fdb09c4d413..4294b97c9ca 100644 --- a/Kernel/CommandLine.cpp +++ b/Kernel/CommandLine.cpp @@ -148,6 +148,19 @@ UNMAP_AFTER_INIT HPETMode CommandLine::hpet_mode() const PANIC("Unknown HPETMode: {}", hpet_mode); } +UNMAP_AFTER_INIT AHCIResetMode CommandLine::ahci_reset_mode() const +{ + const auto ahci_reset_mode = lookup("ahci_reset_mode").value_or("controller"); + if (ahci_reset_mode == "controller") { + return AHCIResetMode::ControllerOnly; + } else if (ahci_reset_mode == "none") { + return AHCIResetMode::None; + } else if (ahci_reset_mode == "complete") { + return AHCIResetMode::Complete; + } + PANIC("Unknown AHCIResetMode: {}", ahci_reset_mode); +} + UNMAP_AFTER_INIT BootMode CommandLine::boot_mode() const { const auto boot_mode = lookup("boot_mode").value_or("graphical"); diff --git a/Kernel/CommandLine.h b/Kernel/CommandLine.h index 75a236bbcd1..91e553026ec 100644 --- a/Kernel/CommandLine.h +++ b/Kernel/CommandLine.h @@ -50,6 +50,12 @@ enum class AcpiFeatureLevel { Disabled, }; +enum class AHCIResetMode { + ControllerOnly, + Complete, + None +}; + class CommandLine { AK_MAKE_ETERNAL; @@ -72,6 +78,7 @@ public: [[nodiscard]] AcpiFeatureLevel acpi_feature_level() const; [[nodiscard]] BootMode boot_mode() const; [[nodiscard]] HPETMode hpet_mode() const; + [[nodiscard]] AHCIResetMode ahci_reset_mode() const; [[nodiscard]] String userspace_init() const; [[nodiscard]] Vector userspace_init_args() const; [[nodiscard]] String root_device() const; diff --git a/Kernel/Storage/AHCIController.cpp b/Kernel/Storage/AHCIController.cpp index 1068ea104aa..d2b0570469a 100644 --- a/Kernel/Storage/AHCIController.cpp +++ b/Kernel/Storage/AHCIController.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -144,11 +145,13 @@ AHCIController::~AHCIController() void AHCIController::initialize() { - if (!reset()) { - dmesgln("{}: AHCI controller reset failed", pci_address()); - return; + if (kernel_command_line().ahci_reset_mode() != AHCIResetMode::None) { + if (!reset()) { + dmesgln("{}: AHCI controller reset failed", pci_address()); + return; + } + dmesgln("{}: AHCI controller reset", pci_address()); } - dmesgln("{}: AHCI controller reset", pci_address()); dbgln("{}: AHCI command list entries count - {}", pci_address(), hba_capabilities().max_command_list_entries_count); hba().control_regs.ghc = 0x80000000; // Ensure that HBA knows we are AHCI aware. PCI::enable_interrupt_line(pci_address()); diff --git a/Kernel/Storage/AHCIPort.cpp b/Kernel/Storage/AHCIPort.cpp index 91d7241a046..b3f1f464bfb 100644 --- a/Kernel/Storage/AHCIPort.cpp +++ b/Kernel/Storage/AHCIPort.cpp @@ -210,6 +210,22 @@ bool AHCIPort::reset() if (!initiate_sata_reset()) { return false; } + return initialize(); +} + +bool AHCIPort::initialize_without_reset() +{ + ScopedSpinLock lock(m_lock); + dmesgln("AHCI Port {}: {}", representative_port_index(), try_disambiguate_sata_status()); + return initialize(); +} + +bool AHCIPort::initialize() +{ + VERIFY(m_lock.is_locked()); + dbgln_if(AHCI_DEBUG, "AHCI Port {}: SIG {:x}", representative_port_index(), static_cast(m_port_registers.sig)); + if (!is_phy_enabled()) + return false; rebase(); power_on(); spin_up(); diff --git a/Kernel/Storage/AHCIPort.h b/Kernel/Storage/AHCIPort.h index 3f26f191ee0..db7aaefc989 100644 --- a/Kernel/Storage/AHCIPort.h +++ b/Kernel/Storage/AHCIPort.h @@ -78,9 +78,13 @@ public: RefPtr connected_device() const { return m_connected_device; } bool reset(); + UNMAP_AFTER_INIT bool initialize_without_reset(); void handle_interrupt(); private: + bool is_phy_enabled() const { return (m_port_registers.ssts & 0xf) == 3; } + bool initialize(); + UNMAP_AFTER_INIT AHCIPort(const AHCIPortHandler&, volatile AHCI::PortRegisters&, u32 port_index); ALWAYS_INLINE void clear_sata_error_register() const; diff --git a/Kernel/Storage/AHCIPortHandler.cpp b/Kernel/Storage/AHCIPortHandler.cpp index 953078b19e8..df5a0ac4dcb 100644 --- a/Kernel/Storage/AHCIPortHandler.cpp +++ b/Kernel/Storage/AHCIPortHandler.cpp @@ -24,6 +24,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include #include namespace Kernel { @@ -46,10 +47,19 @@ AHCIPortHandler::AHCIPortHandler(AHCIController& controller, u8 irq, AHCI::Maske // Clear pending interrupts, if there are any! m_pending_ports_interrupts.set_all(); enable_irq(); + + if (kernel_command_line().ahci_reset_mode() == AHCIResetMode::Complete) { + for (auto index : taken_ports.to_vector()) { + auto port = AHCIPort::create(*this, static_cast(controller.hba().port_regs[index]), index); + m_handled_ports.set(index, port); + port->reset(); + } + return; + } for (auto index : taken_ports.to_vector()) { auto port = AHCIPort::create(*this, static_cast(controller.hba().port_regs[index]), index); m_handled_ports.set(index, port); - port->reset(); + port->initialize_without_reset(); } }