Kernel/Storage: Simplify AHCIPortHandler class

The way AHCIPortHandler held AHCIPorts and even provided them with
physical pages for the ATA identify buffer just felt wrong.
To fix this, AHCIPortHandler is not a ref-counted object anymore. This
solves the big part of the problem, because AHCIPorts can't hold a
reference to this object anymore, only the AHCIController can do that.
Then, most of the responsibilities are shifted to the AHCIController,
making the AHCIPortHandler a handler of port interrupts only.

6 files changed, 98 insertions, 109 deletions

diff --git a/Kernel/Storage/ATA/AHCIController.cpp b/Kernel/Storage/ATA/AHCIController.cpp
index b377ad5c70..40cad6ebcb 100644
--- a/Kernel/Storage/ATA/AHCIController.cpp
+++ b/Kernel/Storage/ATA/AHCIController.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ * Copyright (c) 2021-2022, Liav A. <liavalb@hotmail.co.il>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -10,6 +10,7 @@
 #include <AK/RefPtr.h>
 #include <AK/Types.h>
 #include <Kernel/Bus/PCI/API.h>
+#include <Kernel/CommandLine.h>
 #include <Kernel/Memory/MemoryManager.h>
 #include <Kernel/Storage/ATA/AHCIController.h>
 #include <Kernel/Storage/ATA/AHCIPortHandler.h>
@@ -52,20 +53,16 @@ bool AHCIController::shutdown()
 size_t AHCIController::devices_count() const
 {
     size_t count = 0;
-    for (auto& port_handler : m_handlers) {
-        port_handler.enumerate_ports([&](AHCIPort const& port) {
-            if (port.connected_device())
-                count++;
-        });
+    for (auto port : m_ports) {
+        if (port && port->connected_device())
+            count++;
     }
     return count;
 }
 
 void AHCIController::start_request(ATADevice const& device, AsyncBlockDeviceRequest& request)
 {
-    // FIXME: For now we have one port handler, check all of them...
-    VERIFY(m_handlers.size() > 0);
-    auto port = m_handlers[0].port_at_index(device.ata_address().port);
+    auto port = m_ports[device.ata_address().port];
     VERIFY(port);
     port->start_request(request);
 }
@@ -90,10 +87,16 @@ UNMAP_AFTER_INIT AHCIController::AHCIController(PCI::DeviceIdentifier const& pci
     : ATAController()
     , PCI::Device(pci_device_identifier.address())
     , m_hba_region(default_hba_region())
-    , m_capabilities(capabilities())
+    , m_hba_capabilities(capabilities())
 {
 }
 
+PhysicalAddress AHCIController::get_identify_metadata_physical_region(Badge<AHCIPort>, u32 port_index) const
+{
+    dbgln_if(AHCI_DEBUG, "AHCI Controller @ {}: Get identify metadata physical address of port {} - {}", pci_address(), port_index, (port_index * 512) / PAGE_SIZE);
+    return m_identify_metadata_pages[(port_index * 512) / PAGE_SIZE].paddr().offset((port_index * 512) % PAGE_SIZE);
+}
+
 AHCI::HBADefinedCapabilities AHCIController::capabilities() const
 {
     u32 capabilities = hba().control_regs.cap;
@@ -145,7 +148,7 @@ UNMAP_AFTER_INIT void AHCIController::initialize_hba(PCI::DeviceIdentifier const
         return;
     }
     dmesgln("{}: AHCI controller reset", pci_address());
-    dbgln("{}: AHCI command list entries count - {}", pci_address(), hba_capabilities().max_command_list_entries_count);
+    dbgln("{}: AHCI command list entries count - {}", pci_address(), m_hba_capabilities.max_command_list_entries_count);
 
     u32 version = hba().control_regs.version;
     dbgln_if(AHCI_DEBUG, "{}: AHCI Controller Version = {:#08x}", pci_address(), version);
@@ -155,8 +158,34 @@ UNMAP_AFTER_INIT void AHCIController::initialize_hba(PCI::DeviceIdentifier const
     PCI::enable_bus_mastering(pci_address());
     enable_global_interrupts();
 
-    auto port_handler = AHCIPortHandler::create(*this, pci_device_identifier.interrupt_line().value(), AHCI::MaskedBitField((u32 volatile&)(hba().control_regs.pi))).release_value_but_fixme_should_propagate_errors();
-    m_handlers.append(move(port_handler));
+    auto implemented_ports = AHCI::MaskedBitField((u32 volatile&)(hba().control_regs.pi));
+    m_irq_handler = AHCIPortHandler::create(*this, pci_device_identifier.interrupt_line().value(), implemented_ports).release_value_but_fixme_should_propagate_errors();
+
+    // FIXME: Use the number of ports to determine how many pages we should allocate.
+    for (size_t index = 0; index < (((size_t)AHCI::Limits::MaxPorts * 512) / PAGE_SIZE); index++) {
+        m_identify_metadata_pages.append(MM.allocate_supervisor_physical_page().release_value_but_fixme_should_propagate_errors());
+    }
+
+    if (kernel_command_line().ahci_reset_mode() == AHCIResetMode::Aggressive) {
+        for (auto index : implemented_ports.to_vector()) {
+            auto port = AHCIPort::create(*this, m_hba_capabilities, static_cast<volatile AHCI::PortRegisters&>(hba().port_regs[index]), index).release_value_but_fixme_should_propagate_errors();
+            m_ports[index] = port;
+            port->reset();
+        }
+        return;
+    }
+    for (auto index : implemented_ports.to_vector()) {
+        auto port = AHCIPort::create(*this, m_hba_capabilities, static_cast<volatile AHCI::PortRegisters&>(hba().port_regs[index]), index).release_value_but_fixme_should_propagate_errors();
+        m_ports[index] = port;
+        port->initialize_without_reset();
+    }
+}
+
+void AHCIController::handle_interrupt_for_port(Badge<AHCIPortHandler>, u32 port_index) const
+{
+    auto port = m_ports[port_index];
+    VERIFY(port);
+    port->handle_interrupt();
 }
 
 void AHCIController::disable_global_interrupts() const
@@ -170,16 +199,10 @@ void AHCIController::enable_global_interrupts() const
 
 RefPtr<StorageDevice> AHCIController::device_by_port(u32 port_index) const
 {
-    for (auto& port_handler : m_handlers) {
-        if (!port_handler.is_responsible_for_port_index(port_index))
-            continue;
-
-        auto port = port_handler.port_at_index(port_index);
-        if (!port)
-            return nullptr;
-        return port->connected_device();
-    }
-    return nullptr;
+    auto port = m_ports[port_index];
+    if (!port)
+        return {};
+    return port->connected_device();
 }
 
 RefPtr<StorageDevice> AHCIController::device(u32 index) const
diff --git a/Kernel/Storage/ATA/AHCIController.h b/Kernel/Storage/ATA/AHCIController.h
index 765b97c5a6..451fcf6a60 100644
--- a/Kernel/Storage/ATA/AHCIController.h
+++ b/Kernel/Storage/ATA/AHCIController.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ * Copyright (c) 2021-2022, Liav A. <liavalb@hotmail.co.il>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -22,7 +22,6 @@ class AHCIPort;
 class AHCIController final : public ATAController
     , public PCI::Device {
     friend class AHCIPortHandler;
-    friend class AHCIPort;
 
 public:
     static NonnullRefPtr<AHCIController> initialize(PCI::DeviceIdentifier const& pci_device_identifier);
@@ -35,7 +34,8 @@ public:
     virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
 
-    const AHCI::HBADefinedCapabilities& hba_capabilities() const { return m_capabilities; };
+    PhysicalAddress get_identify_metadata_physical_region(Badge<AHCIPort>, u32 port_index) const;
+    void handle_interrupt_for_port(Badge<AHCIPortHandler>, u32 port_index) const;
 
 private:
     void disable_global_interrupts() const;
@@ -51,8 +51,12 @@ private:
     NonnullOwnPtr<Memory::Region> default_hba_region() const;
     volatile AHCI::HBA& hba() const;
 
+    NonnullRefPtrVector<Memory::PhysicalPage> m_identify_metadata_pages;
+    Array<RefPtr<AHCIPort>, 32> m_ports;
     NonnullOwnPtr<Memory::Region> m_hba_region;
-    AHCI::HBADefinedCapabilities m_capabilities;
-    NonnullRefPtrVector<AHCIPortHandler> m_handlers;
+    AHCI::HBADefinedCapabilities m_hba_capabilities;
+
+    // FIXME: There could be multiple IRQ (MSI) handlers for AHCI. Find a way to use all of them.
+    OwnPtr<AHCIPortHandler> m_irq_handler;
 };
 }
diff --git a/Kernel/Storage/ATA/AHCIPort.cpp b/Kernel/Storage/ATA/AHCIPort.cpp
index 6eb5d74b8f..619c963f15 100644
--- a/Kernel/Storage/ATA/AHCIPort.cpp
+++ b/Kernel/Storage/ATA/AHCIPort.cpp
@@ -20,9 +20,9 @@
 
 namespace Kernel {
 
-UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AHCIPort>> AHCIPort::create(AHCIPortHandler const& handler, volatile AHCI::PortRegisters& registers, u32 port_index)
+UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AHCIPort>> AHCIPort::create(AHCIController const& controller, AHCI::HBADefinedCapabilities hba_capabilities, volatile AHCI::PortRegisters& registers, u32 port_index)
 {
-    auto port = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AHCIPort(handler, registers, port_index)));
+    auto port = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AHCIPort(controller, hba_capabilities, registers, port_index)));
     TRY(port->allocate_resources_and_initialize_ports());
     return port;
 }
@@ -53,10 +53,11 @@ ErrorOr<void> AHCIPort::allocate_resources_and_initialize_ports()
     return {};
 }
 
-UNMAP_AFTER_INIT AHCIPort::AHCIPort(AHCIPortHandler const& handler, volatile AHCI::PortRegisters& registers, u32 port_index)
+UNMAP_AFTER_INIT AHCIPort::AHCIPort(AHCIController const& controller, AHCI::HBADefinedCapabilities hba_capabilities, volatile AHCI::PortRegisters& registers, u32 port_index)
     : m_port_index(port_index)
+    , m_hba_capabilities(hba_capabilities)
     , m_port_registers(registers)
-    , m_parent_handler(handler)
+    , m_parent_controller(controller)
     , m_interrupt_status((u32 volatile&)m_port_registers.is)
     , m_interrupt_enable((u32 volatile&)m_port_registers.ie)
 {
@@ -177,8 +178,14 @@ void AHCIPort::recover_from_fatal_error()
 {
     MutexLocker locker(m_lock);
     SpinlockLocker lock(m_hard_lock);
-    dmesgln("{}: AHCI Port {} fatal error, shutting down!", m_parent_handler->hba_controller()->pci_address(), representative_port_index());
-    dmesgln("{}: AHCI Port {} fatal error, SError {}", m_parent_handler->hba_controller()->pci_address(), representative_port_index(), (u32)m_port_registers.serr);
+    RefPtr<AHCIController> controller = m_parent_controller.strong_ref();
+    if (!controller) {
+        dmesgln("AHCI Port {}: fatal error, controller not available", representative_port_index());
+        return;
+    }
+
+    dmesgln("{}: AHCI Port {} fatal error, shutting down!", controller->pci_address(), representative_port_index());
+    dmesgln("{}: AHCI Port {} fatal error, SError {}", controller->pci_address(), representative_port_index(), (u32)m_port_registers.serr);
     stop_command_list_processing();
     stop_fis_receiving();
     m_interrupt_enable.clear();
@@ -319,8 +326,12 @@ bool AHCIPort::initialize()
     size_t logical_sector_size = 512;
     size_t physical_sector_size = 512;
     u64 max_addressable_sector = 0;
+    RefPtr<AHCIController> controller = m_parent_controller.strong_ref();
+    if (!controller)
+        return false;
+
     if (identify_device()) {
-        auto identify_block = Memory::map_typed<ATAIdentifyBlock>(m_parent_handler->get_identify_metadata_physical_region(m_port_index)).release_value_but_fixme_should_propagate_errors();
+        auto identify_block = Memory::map_typed<ATAIdentifyBlock>(controller->get_identify_metadata_physical_region({}, m_port_index)).release_value_but_fixme_should_propagate_errors();
         // Check if word 106 is valid before using it!
         if ((identify_block->physical_sector_size_to_logical_sector_size >> 14) == 1) {
             if (identify_block->physical_sector_size_to_logical_sector_size & (1 << 12)) {
@@ -345,7 +356,7 @@ bool AHCIPort::initialize()
 
         // FIXME: We don't support ATAPI devices yet, so for now we don't "create" them
         if (!is_atapi_attached()) {
-            m_connected_device = ATADiskDevice::create(m_parent_handler->hba_controller(), { m_port_index, 0 }, 0, logical_sector_size, max_addressable_sector);
+            m_connected_device = ATADiskDevice::create(*controller, { m_port_index, 0 }, 0, logical_sector_size, max_addressable_sector);
         } else {
             dbgln("AHCI Port {}: Ignoring ATAPI devices for now as we don't currently support them.", representative_port_index());
         }
@@ -647,6 +658,10 @@ bool AHCIPort::identify_device()
     if (!spin_until_ready())
         return false;
 
+    RefPtr<AHCIController> controller = m_parent_controller.strong_ref();
+    if (!controller)
+        return false;
+
     auto unused_command_header = try_to_find_unused_command_header();
     VERIFY(unused_command_header.has_value());
     auto* command_list_entries = (volatile AHCI::CommandHeader*)m_command_list_region->vaddr().as_ptr();
@@ -663,7 +678,7 @@ bool AHCIPort::identify_device()
     auto& command_table = *(volatile AHCI::CommandTable*)command_table_region->vaddr().as_ptr();
     memset(const_cast<u8*>(command_table.command_fis), 0, 64);
     command_table.descriptors[0].base_high = 0;
-    command_table.descriptors[0].base_low = m_parent_handler->get_identify_metadata_physical_region(m_port_index).get();
+    command_table.descriptors[0].base_low = controller->get_identify_metadata_physical_region({}, m_port_index).get();
     command_table.descriptors[0].byte_count = 512 - 1;
     auto& fis = *(volatile FIS::HostToDevice::Register*)command_table.command_fis;
     fis.header.fis_type = (u8)FIS::Type::RegisterHostToDevice;
@@ -799,8 +814,8 @@ void AHCIPort::spin_up() const
 {
     VERIFY(m_lock.is_locked());
     VERIFY(m_hard_lock.is_locked());
-    dbgln_if(AHCI_DEBUG, "AHCI Port {}: Spin up. Staggered spin up? {}", representative_port_index(), m_parent_handler->hba_capabilities().staggered_spin_up_supported);
-    if (!m_parent_handler->hba_capabilities().staggered_spin_up_supported)
+    dbgln_if(AHCI_DEBUG, "AHCI Port {}: Spin up. Staggered spin up? {}", representative_port_index(), m_hba_capabilities.staggered_spin_up_supported);
+    if (!m_hba_capabilities.staggered_spin_up_supported)
         return;
     dbgln_if(AHCI_DEBUG, "AHCI Port {}: Spinning up device.", representative_port_index());
     m_port_registers.cmd = m_port_registers.cmd | (1 << 1);
diff --git a/Kernel/Storage/ATA/AHCIPort.h b/Kernel/Storage/ATA/AHCIPort.h
index 70cc1d1f45..4f3686ad5b 100644
--- a/Kernel/Storage/ATA/AHCIPort.h
+++ b/Kernel/Storage/ATA/AHCIPort.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ * Copyright (c) 2021-2022, Liav A. <liavalb@hotmail.co.il>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -33,11 +33,10 @@ class AHCIPortHandler;
 class AHCIPort
     : public RefCounted<AHCIPort>
     , public Weakable<AHCIPort> {
-    friend class AHCIPortHandler;
     friend class AHCIController;
 
 public:
-    static ErrorOr<NonnullRefPtr<AHCIPort>> create(AHCIPortHandler const&, volatile AHCI::PortRegisters&, u32 port_index);
+    static ErrorOr<NonnullRefPtr<AHCIPort>> create(AHCIController const&, AHCI::HBADefinedCapabilities, volatile AHCI::PortRegisters&, u32 port_index);
 
     u32 port_index() const { return m_port_index; }
     u32 representative_port_index() const { return port_index() + 1; }
@@ -56,7 +55,7 @@ private:
     bool is_phy_enabled() const { return (m_port_registers.ssts & 0xf) == 3; }
     bool initialize();
 
-    AHCIPort(AHCIPortHandler const&, volatile AHCI::PortRegisters&, u32 port_index);
+    AHCIPort(AHCIController const&, AHCI::HBADefinedCapabilities, volatile AHCI::PortRegisters&, u32 port_index);
 
     ALWAYS_INLINE void clear_sata_error_register() const;
 
@@ -119,8 +118,14 @@ private:
     RefPtr<ATADevice> m_connected_device;
 
     u32 m_port_index;
+
+    // Note: Ideally the AHCIController should be the only object to hold this data
+    // but because using the m_parent_controller means we need to take a strong ref,
+    // it's probably better to just "cache" this here instead.
+    AHCI::HBADefinedCapabilities const m_hba_capabilities;
+
     volatile AHCI::PortRegisters& m_port_registers;
-    NonnullRefPtr<AHCIPortHandler> m_parent_handler;
+    WeakPtr<AHCIController> m_parent_controller;
     AHCI::PortInterruptStatusBitField m_interrupt_status;
     AHCI::PortInterruptEnableBitField m_interrupt_enable;
 
diff --git a/Kernel/Storage/ATA/AHCIPortHandler.cpp b/Kernel/Storage/ATA/AHCIPortHandler.cpp
index d9a639fcca..c5bc230a62 100644
--- a/Kernel/Storage/ATA/AHCIPortHandler.cpp
+++ b/Kernel/Storage/ATA/AHCIPortHandler.cpp
@@ -1,46 +1,25 @@
 /*
- * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ * Copyright (c) 2021-2022, Liav A. <liavalb@hotmail.co.il>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <Kernel/CommandLine.h>
 #include <Kernel/Storage/ATA/AHCIPortHandler.h>
 
 namespace Kernel {
 
-UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AHCIPortHandler>> AHCIPortHandler::create(AHCIController& controller, u8 irq, AHCI::MaskedBitField taken_ports)
+UNMAP_AFTER_INIT ErrorOr<NonnullOwnPtr<AHCIPortHandler>> AHCIPortHandler::create(AHCIController& controller, u8 irq, AHCI::MaskedBitField taken_ports)
 {
-    auto port_handler = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AHCIPortHandler(controller, irq, taken_ports)));
-    // FIXME: Propagate errors from this method too.
+    auto port_handler = TRY(adopt_nonnull_own_or_enomem(new (nothrow) AHCIPortHandler(controller, irq, taken_ports)));
     port_handler->allocate_resources_and_initialize_ports();
     return port_handler;
 }
 
 void AHCIPortHandler::allocate_resources_and_initialize_ports()
 {
-    // FIXME: Use the number of taken ports to determine how many pages we should allocate.
-    for (size_t index = 0; index < (((size_t)AHCI::Limits::MaxPorts * 512) / PAGE_SIZE); index++) {
-        m_identify_metadata_pages.append(MM.allocate_supervisor_physical_page().release_value_but_fixme_should_propagate_errors());
-    }
-
     // Clear pending interrupts, if there are any!
     m_pending_ports_interrupts.set_all();
     enable_irq();
-
-    if (kernel_command_line().ahci_reset_mode() == AHCIResetMode::Aggressive) {
-        for (auto index : m_taken_ports.to_vector()) {
-            auto port = AHCIPort::create(*this, static_cast<volatile AHCI::PortRegisters&>(m_parent_controller->hba().port_regs[index]), index).release_value_but_fixme_should_propagate_errors();
-            m_handled_ports[index] = port;
-            port->reset();
-        }
-        return;
-    }
-    for (auto index : m_taken_ports.to_vector()) {
-        auto port = AHCIPort::create(*this, static_cast<volatile AHCI::PortRegisters&>(m_parent_controller->hba().port_regs[index]), index).release_value_but_fixme_should_propagate_errors();
-        m_handled_ports[index] = port;
-        port->initialize_without_reset();
-    }
 }
 
 UNMAP_AFTER_INIT AHCIPortHandler::AHCIPortHandler(AHCIController& controller, u8 irq, AHCI::MaskedBitField taken_ports)
@@ -52,36 +31,11 @@ UNMAP_AFTER_INIT AHCIPortHandler::AHCIPortHandler(AHCIController& controller, u8
     dbgln_if(AHCI_DEBUG, "AHCI Port Handler: IRQ {}", irq);
 }
 
-void AHCIPortHandler::enumerate_ports(Function<void(AHCIPort const&)> callback) const
-{
-    for (auto port : m_handled_ports) {
-        if (port)
-            callback(*port);
-    }
-}
-
-RefPtr<AHCIPort> AHCIPortHandler::port_at_index(u32 port_index) const
-{
-    VERIFY(m_taken_ports.is_set_at(port_index));
-    return m_handled_ports[port_index];
-}
-
-PhysicalAddress AHCIPortHandler::get_identify_metadata_physical_region(u32 port_index) const
-{
-    dbgln_if(AHCI_DEBUG, "AHCI Port Handler: Get identify metadata physical address of port {} - {}", port_index, (port_index * 512) / PAGE_SIZE);
-    return m_identify_metadata_pages[(port_index * 512) / PAGE_SIZE].paddr().offset((port_index * 512) % PAGE_SIZE);
-}
-
 AHCI::MaskedBitField AHCIPortHandler::create_pending_ports_interrupts_bitfield() const
 {
     return AHCI::MaskedBitField((u32 volatile&)m_parent_controller->hba().control_regs.is, m_taken_ports.bit_mask());
 }
 
-AHCI::HBADefinedCapabilities AHCIPortHandler::hba_capabilities() const
-{
-    return m_parent_controller->hba_capabilities();
-}
-
 AHCIPortHandler::~AHCIPortHandler() = default;
 
 bool AHCIPortHandler::handle_irq(RegisterState const&)
@@ -90,10 +44,8 @@ bool AHCIPortHandler::handle_irq(RegisterState const&)
     if (m_pending_ports_interrupts.is_zeroed())
         return false;
     for (auto port_index : m_pending_ports_interrupts.to_vector()) {
-        auto port = m_handled_ports[port_index];
-        VERIFY(port);
         dbgln_if(AHCI_DEBUG, "AHCI Port Handler: Handling IRQ for port {}", port_index);
-        port->handle_interrupt();
+        m_parent_controller->handle_interrupt_for_port({}, port_index);
         // We do this to clear the pending interrupt after we handled it.
         m_pending_ports_interrupts.set_at(port_index);
     }
diff --git a/Kernel/Storage/ATA/AHCIPortHandler.h b/Kernel/Storage/ATA/AHCIPortHandler.h
index 63251caba9..8025a21d99 100644
--- a/Kernel/Storage/ATA/AHCIPortHandler.h
+++ b/Kernel/Storage/ATA/AHCIPortHandler.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ * Copyright (c) 2021-2022, Liav A. <liavalb@hotmail.co.il>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -26,20 +26,15 @@ class AsyncBlockDeviceRequest;
 
 class AHCIController;
 class AHCIPort;
-class AHCIPortHandler final : public RefCounted<AHCIPortHandler>
-    , public IRQHandler {
+class AHCIPortHandler final : public IRQHandler {
     friend class AHCIController;
 
 public:
-    static ErrorOr<NonnullRefPtr<AHCIPortHandler>> create(AHCIController&, u8 irq, AHCI::MaskedBitField taken_ports);
+    static ErrorOr<NonnullOwnPtr<AHCIPortHandler>> create(AHCIController&, u8 irq, AHCI::MaskedBitField taken_ports);
     virtual ~AHCIPortHandler() override;
 
     virtual StringView purpose() const override { return "SATA Port Handler"sv; }
 
-    AHCI::HBADefinedCapabilities hba_capabilities() const;
-    NonnullRefPtr<AHCIController> hba_controller() const { return m_parent_controller; }
-    PhysicalAddress get_identify_metadata_physical_region(u32 port_index) const;
-
     bool is_responsible_for_port_index(u32 port_index) const { return m_taken_ports.is_set_at(port_index); }
 
 private:
@@ -57,13 +52,8 @@ private:
 
     AHCI::MaskedBitField create_pending_ports_interrupts_bitfield() const;
 
-    void enumerate_ports(Function<void(AHCIPort const&)> callback) const;
-    RefPtr<AHCIPort> port_at_index(u32 port_index) const;
-
     // Data members
-    Array<RefPtr<AHCIPort>, 32> m_handled_ports;
     NonnullRefPtr<AHCIController> m_parent_controller;
-    NonnullRefPtrVector<Memory::PhysicalPage> m_identify_metadata_pages;
     AHCI::MaskedBitField m_taken_ports;
     AHCI::MaskedBitField m_pending_ports_interrupts;
 };