Kernel/Storage: Move Identify page allocation to the AHCIPort class

Instead of doing this in a parent class like the AHCIController, let's
do that directly in the AHCIPort class as that class is the only user of
these sort of physical pages. While it seems like we waste an entire 4KB
of physical RAM for each allocation, this could serve us later on if we
want to fetch other types of logs from the ATA device.

4 files changed, 14 insertions, 21 deletions

diff --git a/Kernel/Storage/ATA/AHCIController.cpp b/Kernel/Storage/ATA/AHCIController.cpp
index 4ef3ab987d..193118de75 100644
--- a/Kernel/Storage/ATA/AHCIController.cpp
+++ b/Kernel/Storage/ATA/AHCIController.cpp
@@ -91,12 +91,6 @@ UNMAP_AFTER_INIT AHCIController::AHCIController(PCI::DeviceIdentifier const& pci
 {
 }
 
-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;
@@ -161,11 +155,6 @@ UNMAP_AFTER_INIT void AHCIController::initialize_hba(PCI::DeviceIdentifier const
     auto implemented_ports = AHCI::MaskedBitField((u32 volatile&)(hba().control_regs.pi));
     m_irq_handler = AHCIInterruptHandler::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();
diff --git a/Kernel/Storage/ATA/AHCIController.h b/Kernel/Storage/ATA/AHCIController.h
index 2be4ff7c5f..35e49188d6 100644
--- a/Kernel/Storage/ATA/AHCIController.h
+++ b/Kernel/Storage/ATA/AHCIController.h
@@ -34,7 +34,6 @@ public:
     virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
 
-    PhysicalAddress get_identify_metadata_physical_region(Badge<AHCIPort>, u32 port_index) const;
     void handle_interrupt_for_port(Badge<AHCIInterruptHandler>, u32 port_index) const;
 
 private:
@@ -51,7 +50,6 @@ 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_hba_capabilities;
diff --git a/Kernel/Storage/ATA/AHCIPort.cpp b/Kernel/Storage/ATA/AHCIPort.cpp
index 619c963f15..cb688fc973 100644
--- a/Kernel/Storage/ATA/AHCIPort.cpp
+++ b/Kernel/Storage/ATA/AHCIPort.cpp
@@ -22,7 +22,8 @@ namespace Kernel {
 
 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(controller, hba_capabilities, registers, port_index)));
+    auto identify_buffer_page = MUST(MM.allocate_supervisor_physical_page());
+    auto port = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AHCIPort(controller, move(identify_buffer_page), hba_capabilities, registers, port_index)));
     TRY(port->allocate_resources_and_initialize_ports());
     return port;
 }
@@ -53,9 +54,10 @@ ErrorOr<void> AHCIPort::allocate_resources_and_initialize_ports()
     return {};
 }
 
-UNMAP_AFTER_INIT AHCIPort::AHCIPort(AHCIController const& controller, AHCI::HBADefinedCapabilities hba_capabilities, volatile AHCI::PortRegisters& registers, u32 port_index)
+UNMAP_AFTER_INIT AHCIPort::AHCIPort(AHCIController const& controller, NonnullRefPtr<Memory::PhysicalPage> identify_buffer_page, AHCI::HBADefinedCapabilities hba_capabilities, volatile AHCI::PortRegisters& registers, u32 port_index)
     : m_port_index(port_index)
     , m_hba_capabilities(hba_capabilities)
+    , m_identify_buffer_page(move(identify_buffer_page))
     , m_port_registers(registers)
     , m_parent_controller(controller)
     , m_interrupt_status((u32 volatile&)m_port_registers.is)
@@ -326,12 +328,9 @@ 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>(controller->get_identify_metadata_physical_region({}, m_port_index)).release_value_but_fixme_should_propagate_errors();
+        auto identify_block = Memory::map_typed<ATAIdentifyBlock>(m_identify_buffer_page->paddr()).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)) {
@@ -356,6 +355,11 @@ bool AHCIPort::initialize()
 
         // FIXME: We don't support ATAPI devices yet, so for now we don't "create" them
         if (!is_atapi_attached()) {
+            RefPtr<AHCIController> controller = m_parent_controller.strong_ref();
+            if (!controller) {
+                dmesgln("AHCI Port {}: Device found, but parent controller is not available, abort.", representative_port_index());
+                return false;
+            }
             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());
@@ -678,7 +682,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 = controller->get_identify_metadata_physical_region({}, m_port_index).get();
+    command_table.descriptors[0].base_low = m_identify_buffer_page->paddr().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;
diff --git a/Kernel/Storage/ATA/AHCIPort.h b/Kernel/Storage/ATA/AHCIPort.h
index 7648b29f14..0bc5467a95 100644
--- a/Kernel/Storage/ATA/AHCIPort.h
+++ b/Kernel/Storage/ATA/AHCIPort.h
@@ -55,7 +55,7 @@ private:
     bool is_phy_enabled() const { return (m_port_registers.ssts & 0xf) == 3; }
     bool initialize();
 
-    AHCIPort(AHCIController const&, AHCI::HBADefinedCapabilities, volatile AHCI::PortRegisters&, u32 port_index);
+    AHCIPort(AHCIController const&, NonnullRefPtr<Memory::PhysicalPage> identify_buffer_page, AHCI::HBADefinedCapabilities, volatile AHCI::PortRegisters&, u32 port_index);
 
     ALWAYS_INLINE void clear_sata_error_register() const;
 
@@ -124,6 +124,8 @@ private:
     // it's probably better to just "cache" this here instead.
     AHCI::HBADefinedCapabilities const m_hba_capabilities;
 
+    NonnullRefPtr<Memory::PhysicalPage> m_identify_buffer_page;
+
     volatile AHCI::PortRegisters& m_port_registers;
     WeakPtr<AHCIController> m_parent_controller;
     AHCI::PortInterruptStatusBitField m_interrupt_status;