From d431e4cd010de0a8eefd441e7b8f688a0282a7e0 Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 29 May 2021 01:18:45 +0300 Subject: [PATCH] Kernel/Storage: Remove the None option from AHCI reset policy This was proved to be a problematic option. I tested this option on bare metal AHCI controller, and if we didn't reset the controller, the firmware (SeaBIOS) could leave the controller state not clean, so an plugged device signature was in place although the specific port had no plugged device after rebooting. Therefore, we need to ensure we use the controller in a clean state always. In addition to that, the Complete option was renamed to Aggressive, as it represents better the consequences of choosing this option. --- Kernel/CommandLine.cpp | 6 ++---- Kernel/CommandLine.h | 3 +-- Kernel/Storage/AHCIController.cpp | 10 ++++------ Kernel/Storage/AHCIPortHandler.cpp | 2 +- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/Kernel/CommandLine.cpp b/Kernel/CommandLine.cpp index 98684058e7..441964d1ef 100644 --- a/Kernel/CommandLine.cpp +++ b/Kernel/CommandLine.cpp @@ -178,10 +178,8 @@ 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; + } else if (ahci_reset_mode == "aggressive") { + return AHCIResetMode::Aggressive; } PANIC("Unknown AHCIResetMode: {}", ahci_reset_mode); } diff --git a/Kernel/CommandLine.h b/Kernel/CommandLine.h index 0c40cdb0b5..8820072386 100644 --- a/Kernel/CommandLine.h +++ b/Kernel/CommandLine.h @@ -38,8 +38,7 @@ enum class PCIAccessLevel { enum class AHCIResetMode { ControllerOnly, - Complete, - None + Aggressive, }; class CommandLine { diff --git a/Kernel/Storage/AHCIController.cpp b/Kernel/Storage/AHCIController.cpp index 7e2731296b..4a2044f556 100644 --- a/Kernel/Storage/AHCIController.cpp +++ b/Kernel/Storage/AHCIController.cpp @@ -137,13 +137,11 @@ AHCIController::~AHCIController() void AHCIController::initialize() { - 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()); + if (!reset()) { + dmesgln("{}: AHCI controller reset failed", pci_address()); + return; } + dmesgln("{}: AHCI controller reset", pci_address()); dbgln("{}: AHCI command list entries count - {}", pci_address(), hba_capabilities().max_command_list_entries_count); u32 version = hba().control_regs.version; diff --git a/Kernel/Storage/AHCIPortHandler.cpp b/Kernel/Storage/AHCIPortHandler.cpp index 51ceb329e4..e3dcaa9bcf 100644 --- a/Kernel/Storage/AHCIPortHandler.cpp +++ b/Kernel/Storage/AHCIPortHandler.cpp @@ -31,7 +31,7 @@ AHCIPortHandler::AHCIPortHandler(AHCIController& controller, u8 irq, AHCI::Maske m_pending_ports_interrupts.set_all(); enable_irq(); - if (kernel_command_line().ahci_reset_mode() == AHCIResetMode::Complete) { + if (kernel_command_line().ahci_reset_mode() == AHCIResetMode::Aggressive) { 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);