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, 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(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, u32 port_index) const; void handle_interrupt_for_port(Badge, u32 port_index) const; private: @@ -51,7 +50,6 @@ private: NonnullOwnPtr default_hba_region() const; volatile AHCI::HBA& hba() const; - NonnullRefPtrVector m_identify_metadata_pages; Array, 32> m_ports; NonnullOwnPtr 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> 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 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 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 controller = m_parent_controller.strong_ref(); - if (!controller) - return false; if (identify_device()) { - auto identify_block = Memory::map_typed(controller->get_identify_metadata_physical_region({}, m_port_index)).release_value_but_fixme_should_propagate_errors(); + auto identify_block = Memory::map_typed(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 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(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 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 m_identify_buffer_page; + volatile AHCI::PortRegisters& m_port_registers; WeakPtr m_parent_controller; AHCI::PortInterruptStatusBitField m_interrupt_status;