From 428d4ae337fa1fa78a0cba80debe8ab1cae34761 Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 4 Feb 2022 22:11:50 +0200 Subject: [PATCH] Kernel/PCI: Break early of controller iteration over devices in OOM case This is mainly useful when adding an HostController but due to OOM condition, we abort temporary Vector insertion of a DeviceIdentifier and then exit the iteration loop to report back the error if occured. --- Kernel/Bus/PCI/Access.cpp | 10 +++++++--- Kernel/Bus/PCI/Controller/HostBridge.cpp | 8 ++++---- Kernel/Bus/PCI/Controller/HostBridge.h | 8 ++++---- Kernel/Bus/PCI/Controller/HostController.h | 2 +- Kernel/Storage/StorageManagement.cpp | 3 ++- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Kernel/Bus/PCI/Access.cpp b/Kernel/Bus/PCI/Access.cpp index 12d1c42d1e..cc0886aa18 100644 --- a/Kernel/Bus/PCI/Access.cpp +++ b/Kernel/Bus/PCI/Access.cpp @@ -133,11 +133,14 @@ ErrorOr Access::add_host_controller_and_enumerate_attached_devices(Nonnull // definitely before enumerating devices behing that. m_host_controllers.set(domain_number, move(controller)); ErrorOr expansion_result; - m_host_controllers.get(domain_number).value()->enumerate_attached_devices([&](DeviceIdentifier const& device_identifier) -> void { + m_host_controllers.get(domain_number).value()->enumerate_attached_devices([&](DeviceIdentifier const& device_identifier) -> IterationDecision { m_device_identifiers.append(device_identifier); auto result = device_identifiers_behind_host_controller.try_append(device_identifier); - if (result.is_error()) + if (result.is_error()) { expansion_result = result; + return IterationDecision::Break; + } + return IterationDecision::Continue; }); if (expansion_result.is_error()) return expansion_result; @@ -166,8 +169,9 @@ UNMAP_AFTER_INIT void Access::rescan_hardware() SpinlockLocker scan_locker(m_scan_lock); VERIFY(m_device_identifiers.is_empty()); for (auto it = m_host_controllers.begin(); it != m_host_controllers.end(); ++it) { - (*it).value->enumerate_attached_devices([this](DeviceIdentifier device_identifier) -> void { + (*it).value->enumerate_attached_devices([this](DeviceIdentifier device_identifier) -> IterationDecision { m_device_identifiers.append(device_identifier); + return IterationDecision::Continue; }); } } diff --git a/Kernel/Bus/PCI/Controller/HostBridge.cpp b/Kernel/Bus/PCI/Controller/HostBridge.cpp index 20406414cf..6767967262 100644 --- a/Kernel/Bus/PCI/Controller/HostBridge.cpp +++ b/Kernel/Bus/PCI/Controller/HostBridge.cpp @@ -50,7 +50,7 @@ UNMAP_AFTER_INIT Vector HostBridge::get_capabilities_for_function(Bu return capabilities; } -UNMAP_AFTER_INIT void HostBridge::enumerate_functions(Function const& callback, BusNumber bus, DeviceNumber device, FunctionNumber function, bool recursive_search_into_bridges) +UNMAP_AFTER_INIT void HostBridge::enumerate_functions(Function const& callback, BusNumber bus, DeviceNumber device, FunctionNumber function, bool recursive_search_into_bridges) { dbgln_if(PCI_DEBUG, "PCI: Enumerating function, bus={}, device={}, function={}", bus, device, function); Address address(domain_number(), bus.value(), device.value(), function.value()); @@ -79,7 +79,7 @@ UNMAP_AFTER_INIT void HostBridge::enumerate_functions(Function const& callback, BusNumber bus, DeviceNumber device, bool recursive_search_into_bridges) +UNMAP_AFTER_INIT void HostBridge::enumerate_device(Function const& callback, BusNumber bus, DeviceNumber device, bool recursive_search_into_bridges) { dbgln_if(PCI_DEBUG, "PCI: Enumerating device in bus={}, device={}", bus, device); if (read16_field(bus, device, 0, PCI::RegisterOffset::VENDOR_ID) == PCI::none_value) @@ -93,14 +93,14 @@ UNMAP_AFTER_INIT void HostBridge::enumerate_device(Function const& callback, BusNumber bus, bool recursive_search_into_bridges) +UNMAP_AFTER_INIT void HostBridge::enumerate_bus(Function const& callback, BusNumber bus, bool recursive_search_into_bridges) { dbgln_if(PCI_DEBUG, "PCI: Enumerating bus {}", bus); for (u8 device = 0; device < 32; ++device) enumerate_device(callback, bus, device, recursive_search_into_bridges); } -UNMAP_AFTER_INIT void HostBridge::enumerate_attached_devices(Function callback) +UNMAP_AFTER_INIT void HostBridge::enumerate_attached_devices(Function callback) { VERIFY(Access::the().access_lock().is_locked()); VERIFY(Access::the().scan_lock().is_locked()); diff --git a/Kernel/Bus/PCI/Controller/HostBridge.h b/Kernel/Bus/PCI/Controller/HostBridge.h index ce36a3f3d8..2ba235ba89 100644 --- a/Kernel/Bus/PCI/Controller/HostBridge.h +++ b/Kernel/Bus/PCI/Controller/HostBridge.h @@ -31,7 +31,7 @@ protected: explicit HostBridge(PCI::Domain const&); private: - virtual void enumerate_attached_devices(Function callback) override; + virtual void enumerate_attached_devices(Function callback) override; Bitmap m_enumerated_buses; @@ -41,9 +41,9 @@ private: Optional get_capabilities_pointer_for_function(BusNumber, DeviceNumber, FunctionNumber); Vector get_capabilities_for_function(BusNumber, DeviceNumber, FunctionNumber); - void enumerate_bus(Function const& callback, BusNumber, bool recursive); - void enumerate_functions(Function const& callback, BusNumber, DeviceNumber, FunctionNumber, bool recursive); - void enumerate_device(Function const& callback, BusNumber bus, DeviceNumber device, bool recursive); + void enumerate_bus(Function const& callback, BusNumber, bool recursive); + void enumerate_functions(Function const& callback, BusNumber, DeviceNumber, FunctionNumber, bool recursive); + void enumerate_device(Function const& callback, BusNumber bus, DeviceNumber device, bool recursive); }; } diff --git a/Kernel/Bus/PCI/Controller/HostController.h b/Kernel/Bus/PCI/Controller/HostController.h index 80a10280cf..9089fde3e9 100644 --- a/Kernel/Bus/PCI/Controller/HostController.h +++ b/Kernel/Bus/PCI/Controller/HostController.h @@ -31,7 +31,7 @@ public: u32 domain_number() const { return m_domain.domain_number(); } - virtual void enumerate_attached_devices(Function callback) = 0; + virtual void enumerate_attached_devices(Function callback) = 0; protected: explicit HostController(PCI::Domain const& domain) diff --git a/Kernel/Storage/StorageManagement.cpp b/Kernel/Storage/StorageManagement.cpp index 3ca934d018..f737140fa5 100644 --- a/Kernel/Storage/StorageManagement.cpp +++ b/Kernel/Storage/StorageManagement.cpp @@ -5,6 +5,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -53,7 +54,7 @@ UNMAP_AFTER_INIT void StorageManagement::enumerate_pci_controllers(bool force_pi using SubclassID = PCI::MassStorage::SubclassID; if (!kernel_command_line().disable_physical_storage()) { - MUST(PCI::enumerate([&](PCI::DeviceIdentifier const& device_identifier) { + MUST(PCI::enumerate([&](PCI::DeviceIdentifier const& device_identifier) -> void { if (device_identifier.class_code().value() != to_underlying(PCI::ClassID::MassStorage)) { return; }