1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 23:37:36 +00:00

Kernel/PCI: Don't hold spinlocks when doing fast device enumeration

Instead, hold the lock while we copy the contents to a stack-based
Vector then iterate on it without any locking.

Because we rely on heap allocations, we need to propagate errors back
in case of OOM condition, therefore, both PCI::enumerate API function
and PCI::Access::add_host_controller_and_enumerate_attached_devices use
now a ErrorOr<void> return value to propagate errors. OOM Error can only
occur when enumerating the m_device_identifiers vector under a spinlock
and trying to expand the temporary Vector which will be used locklessly
to actually iterate over the PCI::DeviceIdentifiers objects.
This commit is contained in:
Liav A 2022-02-04 19:48:13 +02:00 committed by Andreas Kling
parent c0ed656c94
commit 3fb289e27d
14 changed files with 69 additions and 42 deletions

View file

@ -17,9 +17,9 @@ u8 read8(Address address, PCI::RegisterOffset field) { return Access::the().read
u16 read16(Address address, PCI::RegisterOffset field) { return Access::the().read16_field(address, to_underlying(field)); }
u32 read32(Address address, PCI::RegisterOffset field) { return Access::the().read32_field(address, to_underlying(field)); }
void enumerate(Function<void(DeviceIdentifier const&)> callback)
ErrorOr<void> enumerate(Function<void(DeviceIdentifier const&)> callback)
{
Access::the().fast_enumerate(callback);
return Access::the().fast_enumerate(callback);
}
DeviceIdentifier get_device_identifier(Address address)

View file

@ -6,6 +6,8 @@
#pragma once
#include <AK/Error.h>
#include <AK/Try.h>
#include <Kernel/Bus/PCI/Definitions.h>
namespace Kernel::PCI {
@ -19,7 +21,7 @@ u32 read32(Address address, PCI::RegisterOffset field);
HardwareID get_hardware_id(PCI::Address);
bool is_io_space_enabled(Address);
void enumerate(Function<void(DeviceIdentifier const&)> callback);
ErrorOr<void> enumerate(Function<void(DeviceIdentifier const&)> callback);
void enable_interrupt_line(Address);
void disable_interrupt_line(Address);
void raw_access(Address, u32, size_t, u32);

View file

@ -117,20 +117,36 @@ UNMAP_AFTER_INIT bool Access::initialize_for_one_pci_domain()
return true;
}
void Access::add_host_controller_and_enumerate_attached_devices(NonnullOwnPtr<HostController> controller, Function<void(DeviceIdentifier const&)> callback)
ErrorOr<void> Access::add_host_controller_and_enumerate_attached_devices(NonnullOwnPtr<HostController> controller, Function<void(DeviceIdentifier const&)> callback)
{
SpinlockLocker locker(m_access_lock);
SpinlockLocker scan_locker(m_scan_lock);
auto domain_number = controller->domain_number();
// Note: We hold the spinlocks for a moment just to ensure we append the
// device identifiers safely. Afterwards, enumeration goes lockless to allow
// IRQs to be fired if necessary.
Vector<DeviceIdentifier> device_identifiers_behind_host_controller;
{
SpinlockLocker locker(m_access_lock);
SpinlockLocker scan_locker(m_scan_lock);
auto domain_number = controller->domain_number();
VERIFY(!m_host_controllers.contains(domain_number));
// Note: We need to register the new controller as soon as possible, and
// definitely before enumerating devices behing that.
m_host_controllers.set(domain_number, move(controller));
m_host_controllers.get(domain_number).value()->enumerate_attached_devices([&](DeviceIdentifier const& device_identifier) -> void {
m_device_identifiers.append(device_identifier);
VERIFY(!m_host_controllers.contains(domain_number));
// Note: We need to register the new controller as soon as possible, and
// definitely before enumerating devices behing that.
m_host_controllers.set(domain_number, move(controller));
ErrorOr<void> expansion_result;
m_host_controllers.get(domain_number).value()->enumerate_attached_devices([&](DeviceIdentifier const& device_identifier) -> void {
m_device_identifiers.append(device_identifier);
auto result = device_identifiers_behind_host_controller.try_append(device_identifier);
if (result.is_error())
expansion_result = result;
});
if (expansion_result.is_error())
return expansion_result;
}
for (auto const& device_identifier : device_identifiers_behind_host_controller) {
callback(device_identifier);
});
}
return {};
}
UNMAP_AFTER_INIT void Access::add_host_controller(NonnullOwnPtr<HostController> controller)
@ -156,13 +172,20 @@ UNMAP_AFTER_INIT void Access::rescan_hardware()
}
}
void Access::fast_enumerate(Function<void(DeviceIdentifier const&)>& callback) const
ErrorOr<void> Access::fast_enumerate(Function<void(DeviceIdentifier const&)>& callback) const
{
SpinlockLocker locker(m_access_lock);
VERIFY(!m_device_identifiers.is_empty());
for (auto const& device_identifier : m_device_identifiers) {
// Note: We hold the m_access_lock for a brief moment just to ensure we get
// a complete Vector in case someone wants to mutate it.
Vector<DeviceIdentifier> device_identifiers;
{
SpinlockLocker locker(m_access_lock);
VERIFY(!m_device_identifiers.is_empty());
TRY(device_identifiers.try_extend(m_device_identifiers));
}
for (auto const& device_identifier : device_identifiers) {
callback(device_identifier);
}
return {};
}
DeviceIdentifier Access::get_device_identifier(Address address) const

View file

@ -7,6 +7,7 @@
#pragma once
#include <AK/Bitmap.h>
#include <AK/Try.h>
#include <AK/Vector.h>
#include <Kernel/Bus/PCI/Controller/HostController.h>
#include <Kernel/Bus/PCI/Definitions.h>
@ -21,7 +22,7 @@ public:
static bool initialize_for_multiple_pci_domains(PhysicalAddress mcfg_table);
static bool initialize_for_one_pci_domain();
void fast_enumerate(Function<void(DeviceIdentifier const&)>&) const;
ErrorOr<void> fast_enumerate(Function<void(DeviceIdentifier const&)>&) const;
void rescan_hardware();
static Access& the();
@ -39,7 +40,7 @@ public:
Spinlock const& scan_lock() const { return m_scan_lock; }
RecursiveSpinlock const& access_lock() const { return m_access_lock; }
void add_host_controller_and_enumerate_attached_devices(NonnullOwnPtr<HostController>, Function<void(DeviceIdentifier const&)> callback);
ErrorOr<void> add_host_controller_and_enumerate_attached_devices(NonnullOwnPtr<HostController>, Function<void(DeviceIdentifier const&)> callback);
private:
u8 read8_field(Address address, RegisterOffset field);

View file

@ -62,9 +62,9 @@ UNMAP_AFTER_INIT void initialize()
PCI::PCIBusSysFSDirectory::initialize();
PCI::enumerate([&](DeviceIdentifier const& device_identifier) {
MUST(PCI::enumerate([&](DeviceIdentifier const& device_identifier) {
dmesgln("{} {}", device_identifier.address(), device_identifier.hardware_id());
});
}));
}
UNMAP_AFTER_INIT bool test_pci_io()

View file

@ -50,10 +50,10 @@ UNMAP_AFTER_INIT void PCIBusSysFSDirectory::initialize()
UNMAP_AFTER_INIT PCIBusSysFSDirectory::PCIBusSysFSDirectory()
: SysFSDirectory(SysFSComponentRegistry::the().buses_directory())
{
PCI::enumerate([&](DeviceIdentifier const& device_identifier) {
MUST(PCI::enumerate([&](DeviceIdentifier const& device_identifier) {
auto pci_device = PCI::PCIDeviceSysFSDirectory::create(*this, device_identifier.address());
m_components.append(pci_device);
});
}));
}
StringView PCIDeviceAttributeSysFSComponent::name() const