diff options
author | Andreas Kling <kling@serenityos.org> | 2022-12-11 18:48:20 +0100 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2022-12-13 11:20:11 +0100 |
commit | 30d3f2789e3478da415ccdc6766872d3d58918e2 (patch) | |
tree | 06bb64b11b320d38a583cb8b4756432ab0d0200d /Kernel/Net | |
parent | 9a849c10ae4f376034cfa34a730b1de5fa343601 (diff) | |
download | serenity-30d3f2789e3478da415ccdc6766872d3d58918e2.zip |
Kernel: Propagate errors during network adapter detection/initialization
When scanning for network adapters, we give each driver a chance to
claim the PCI device and whoever claims it first gets to keep it.
Before this patch, the driver API returned a LockRefPtr<AdapterType>,
which made it impossible to propagate errors that occurred during
detection and/or initialization.
This patch changes the API so that errors can bubble all the way out
the PCI enumeration in NetworkingManagement::initialize() where we
perform all the network adapter auto-detection on boot.
When we eventually start to support hot-plugging network adapter in the
future, it will be even more important to propagate errors instead of
swallowing them.
Importantly, before this patch, some errors were "handled" by panicking
the kernel. This is no longer the case.
7 FIXMEs were killed in the making of this commit. :^)
Diffstat (limited to 'Kernel/Net')
-rw-r--r-- | Kernel/Net/Intel/E1000ENetworkAdapter.cpp | 23 | ||||
-rw-r--r-- | Kernel/Net/Intel/E1000ENetworkAdapter.h | 2 | ||||
-rw-r--r-- | Kernel/Net/Intel/E1000NetworkAdapter.cpp | 23 | ||||
-rw-r--r-- | Kernel/Net/Intel/E1000NetworkAdapter.h | 2 | ||||
-rw-r--r-- | Kernel/Net/NE2000/NetworkAdapter.cpp | 13 | ||||
-rw-r--r-- | Kernel/Net/NE2000/NetworkAdapter.h | 2 | ||||
-rw-r--r-- | Kernel/Net/NetworkingManagement.cpp | 32 | ||||
-rw-r--r-- | Kernel/Net/NetworkingManagement.h | 2 | ||||
-rw-r--r-- | Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp | 13 | ||||
-rw-r--r-- | Kernel/Net/Realtek/RTL8139NetworkAdapter.h | 2 | ||||
-rw-r--r-- | Kernel/Net/Realtek/RTL8168NetworkAdapter.cpp | 15 | ||||
-rw-r--r-- | Kernel/Net/Realtek/RTL8168NetworkAdapter.h | 2 |
12 files changed, 58 insertions, 73 deletions
diff --git a/Kernel/Net/Intel/E1000ENetworkAdapter.cpp b/Kernel/Net/Intel/E1000ENetworkAdapter.cpp index cdbf1fab46..395416d1a7 100644 --- a/Kernel/Net/Intel/E1000ENetworkAdapter.cpp +++ b/Kernel/Net/Intel/E1000ENetworkAdapter.cpp @@ -181,24 +181,19 @@ static bool is_valid_device_id(u16 device_id) } } -UNMAP_AFTER_INIT LockRefPtr<E1000ENetworkAdapter> E1000ENetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT ErrorOr<LockRefPtr<E1000ENetworkAdapter>> E1000ENetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) { if (pci_device_identifier.hardware_id().vendor_id != PCI::VendorID::Intel) - return {}; + return nullptr; if (!is_valid_device_id(pci_device_identifier.hardware_id().device_id)) - return {}; + return nullptr; u8 irq = pci_device_identifier.interrupt_line().value(); - // FIXME: Better propagate errors here - auto interface_name_or_error = NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier); - if (interface_name_or_error.is_error()) - return {}; - auto registers_io_window = IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0).release_value_but_fixme_should_propagate_errors(); - auto adapter = adopt_lock_ref_if_nonnull(new (nothrow) E1000ENetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), interface_name_or_error.release_value())); - if (!adapter) - return {}; - if (adapter->initialize()) - return adapter; - return {}; + auto interface_name = TRY(NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier)); + auto registers_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); + auto adapter = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) E1000ENetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), move(interface_name)))); + if (!adapter->initialize()) + return Error::from_string_literal("E1000ENetworkAdapter: Unable to initialize adapter"); + return adapter; } UNMAP_AFTER_INIT bool E1000ENetworkAdapter::initialize() diff --git a/Kernel/Net/Intel/E1000ENetworkAdapter.h b/Kernel/Net/Intel/E1000ENetworkAdapter.h index 64ef266401..ea62860d1f 100644 --- a/Kernel/Net/Intel/E1000ENetworkAdapter.h +++ b/Kernel/Net/Intel/E1000ENetworkAdapter.h @@ -21,7 +21,7 @@ namespace Kernel { class E1000ENetworkAdapter final : public E1000NetworkAdapter { public: - static LockRefPtr<E1000ENetworkAdapter> try_to_initialize(PCI::DeviceIdentifier const&); + static ErrorOr<LockRefPtr<E1000ENetworkAdapter>> try_to_initialize(PCI::DeviceIdentifier const&); virtual bool initialize() override; diff --git a/Kernel/Net/Intel/E1000NetworkAdapter.cpp b/Kernel/Net/Intel/E1000NetworkAdapter.cpp index d7e5239b9b..9a755569e1 100644 --- a/Kernel/Net/Intel/E1000NetworkAdapter.cpp +++ b/Kernel/Net/Intel/E1000NetworkAdapter.cpp @@ -159,24 +159,19 @@ UNMAP_AFTER_INIT static bool is_valid_device_id(u16 device_id) } } -UNMAP_AFTER_INIT LockRefPtr<E1000NetworkAdapter> E1000NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT ErrorOr<LockRefPtr<E1000NetworkAdapter>> E1000NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) { if (pci_device_identifier.hardware_id().vendor_id != PCI::VendorID::Intel) - return {}; + return nullptr; if (!is_valid_device_id(pci_device_identifier.hardware_id().device_id)) - return {}; + return nullptr; u8 irq = pci_device_identifier.interrupt_line().value(); - // FIXME: Better propagate errors here - auto interface_name_or_error = NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier); - if (interface_name_or_error.is_error()) - return {}; - auto registers_io_window = IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0).release_value_but_fixme_should_propagate_errors(); - auto adapter = adopt_lock_ref_if_nonnull(new (nothrow) E1000NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), interface_name_or_error.release_value())); - if (!adapter) - return {}; - if (adapter->initialize()) - return adapter; - return {}; + auto interface_name = TRY(NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier)); + auto registers_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); + auto adapter = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) E1000NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), move(interface_name)))); + if (!adapter->initialize()) + return Error::from_string_literal("E1000NetworkAdapter: Unable to initialize adapter"); + return adapter; } UNMAP_AFTER_INIT void E1000NetworkAdapter::setup_link() diff --git a/Kernel/Net/Intel/E1000NetworkAdapter.h b/Kernel/Net/Intel/E1000NetworkAdapter.h index 3f40733fbf..d14920dda5 100644 --- a/Kernel/Net/Intel/E1000NetworkAdapter.h +++ b/Kernel/Net/Intel/E1000NetworkAdapter.h @@ -20,7 +20,7 @@ class E1000NetworkAdapter : public NetworkAdapter , public PCI::Device , public IRQHandler { public: - static LockRefPtr<E1000NetworkAdapter> try_to_initialize(PCI::DeviceIdentifier const&); + static ErrorOr<LockRefPtr<E1000NetworkAdapter>> try_to_initialize(PCI::DeviceIdentifier const&); virtual bool initialize(); diff --git a/Kernel/Net/NE2000/NetworkAdapter.cpp b/Kernel/Net/NE2000/NetworkAdapter.cpp index 30dd731c3d..65fba19d1f 100644 --- a/Kernel/Net/NE2000/NetworkAdapter.cpp +++ b/Kernel/Net/NE2000/NetworkAdapter.cpp @@ -138,7 +138,7 @@ struct [[gnu::packed]] received_packet_header { u16 length; }; -UNMAP_AFTER_INIT LockRefPtr<NE2000NetworkAdapter> NE2000NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT ErrorOr<LockRefPtr<NE2000NetworkAdapter>> NE2000NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) { constexpr auto ne2k_ids = Array { PCI::HardwareID { 0x10EC, 0x8029 }, // RealTek RTL-8029(AS) @@ -156,14 +156,11 @@ UNMAP_AFTER_INIT LockRefPtr<NE2000NetworkAdapter> NE2000NetworkAdapter::try_to_i PCI::HardwareID { 0x8c4a, 0x1980 }, // Winbond W89C940 (misprogrammed) }; if (!ne2k_ids.span().contains_slow(pci_device_identifier.hardware_id())) - return {}; + return nullptr; u8 irq = pci_device_identifier.interrupt_line().value(); - // FIXME: Better propagate errors here - auto interface_name_or_error = NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier); - if (interface_name_or_error.is_error()) - return {}; - auto registers_io_window = MUST(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); - return adopt_lock_ref_if_nonnull(new (nothrow) NE2000NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), interface_name_or_error.release_value())); + auto interface_name = TRY(NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier)); + auto registers_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); + return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) NE2000NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), move(interface_name)))); } UNMAP_AFTER_INIT NE2000NetworkAdapter::NE2000NetworkAdapter(PCI::Address address, u8 irq, NonnullOwnPtr<IOWindow> registers_io_window, NonnullOwnPtr<KString> interface_name) diff --git a/Kernel/Net/NE2000/NetworkAdapter.h b/Kernel/Net/NE2000/NetworkAdapter.h index 1460da1614..ee37fdd6a2 100644 --- a/Kernel/Net/NE2000/NetworkAdapter.h +++ b/Kernel/Net/NE2000/NetworkAdapter.h @@ -20,7 +20,7 @@ class NE2000NetworkAdapter final : public NetworkAdapter , public PCI::Device , public IRQHandler { public: - static LockRefPtr<NE2000NetworkAdapter> try_to_initialize(PCI::DeviceIdentifier const&); + static ErrorOr<LockRefPtr<NE2000NetworkAdapter>> try_to_initialize(PCI::DeviceIdentifier const&); virtual ~NE2000NetworkAdapter() override; diff --git a/Kernel/Net/NetworkingManagement.cpp b/Kernel/Net/NetworkingManagement.cpp index e3fe772bf6..2da3aef1e9 100644 --- a/Kernel/Net/NetworkingManagement.cpp +++ b/Kernel/Net/NetworkingManagement.cpp @@ -93,19 +93,19 @@ ErrorOr<NonnullOwnPtr<KString>> NetworkingManagement::generate_interface_name_fr return name; } -UNMAP_AFTER_INIT LockRefPtr<NetworkAdapter> NetworkingManagement::determine_network_device(PCI::DeviceIdentifier const& device_identifier) const +UNMAP_AFTER_INIT ErrorOr<NonnullLockRefPtr<NetworkAdapter>> NetworkingManagement::determine_network_device(PCI::DeviceIdentifier const& device_identifier) const { - if (auto candidate = E1000NetworkAdapter::try_to_initialize(device_identifier); !candidate.is_null()) - return candidate; - if (auto candidate = E1000ENetworkAdapter::try_to_initialize(device_identifier); !candidate.is_null()) - return candidate; - if (auto candidate = RTL8139NetworkAdapter::try_to_initialize(device_identifier); !candidate.is_null()) - return candidate; - if (auto candidate = RTL8168NetworkAdapter::try_to_initialize(device_identifier); !candidate.is_null()) - return candidate; - if (auto candidate = NE2000NetworkAdapter::try_to_initialize(device_identifier); !candidate.is_null()) - return candidate; - return {}; + if (auto candidate = TRY(E1000NetworkAdapter::try_to_initialize(device_identifier))) + return candidate.release_nonnull(); + if (auto candidate = TRY(E1000ENetworkAdapter::try_to_initialize(device_identifier))) + return candidate.release_nonnull(); + if (auto candidate = TRY(RTL8139NetworkAdapter::try_to_initialize(device_identifier))) + return candidate.release_nonnull(); + if (auto candidate = TRY(RTL8168NetworkAdapter::try_to_initialize(device_identifier))) + return candidate.release_nonnull(); + if (auto candidate = TRY(NE2000NetworkAdapter::try_to_initialize(device_identifier))) + return candidate.release_nonnull(); + return Error::from_string_literal("Unsupported network adapter"); } bool NetworkingManagement::initialize() @@ -115,8 +115,12 @@ bool NetworkingManagement::initialize() // Note: PCI class 2 is the class of Network devices if (device_identifier.class_code().value() != 0x02) return; - if (auto adapter = determine_network_device(device_identifier); !adapter.is_null()) - m_adapters.with([&](auto& adapters) { adapters.append(adapter.release_nonnull()); }); + auto result = determine_network_device(device_identifier); + if (result.is_error()) { + dmesgln("Failed to initialize network adapter ({} {}): {}", device_identifier.address(), device_identifier.hardware_id(), result.error()); + return; + } + m_adapters.with([&](auto& adapters) { adapters.append(result.release_value()); }); })); } auto loopback = LoopbackAdapter::try_create(); diff --git a/Kernel/Net/NetworkingManagement.h b/Kernel/Net/NetworkingManagement.h index 2876993dfd..8b8955e919 100644 --- a/Kernel/Net/NetworkingManagement.h +++ b/Kernel/Net/NetworkingManagement.h @@ -40,7 +40,7 @@ public: NonnullLockRefPtr<NetworkAdapter> loopback_adapter() const; private: - LockRefPtr<NetworkAdapter> determine_network_device(PCI::DeviceIdentifier const&) const; + ErrorOr<NonnullLockRefPtr<NetworkAdapter>> determine_network_device(PCI::DeviceIdentifier const&) const; SpinlockProtected<NonnullLockRefPtrVector<NetworkAdapter>> m_adapters { LockRank::None }; LockRefPtr<NetworkAdapter> m_loopback_adapter; diff --git a/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp b/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp index da87dfa8ba..3d2fecdb89 100644 --- a/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp +++ b/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp @@ -112,18 +112,15 @@ namespace Kernel { #define RX_BUFFER_SIZE 32768 #define TX_BUFFER_SIZE PACKET_SIZE_MAX -UNMAP_AFTER_INIT LockRefPtr<RTL8139NetworkAdapter> RTL8139NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT ErrorOr<LockRefPtr<RTL8139NetworkAdapter>> RTL8139NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) { constexpr PCI::HardwareID rtl8139_id = { 0x10EC, 0x8139 }; if (pci_device_identifier.hardware_id() != rtl8139_id) - return {}; + return nullptr; u8 irq = pci_device_identifier.interrupt_line().value(); - // FIXME: Better propagate errors here - auto interface_name_or_error = NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier); - if (interface_name_or_error.is_error()) - return {}; - auto registers_io_window = MUST(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); - return adopt_lock_ref_if_nonnull(new (nothrow) RTL8139NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), interface_name_or_error.release_value())); + auto interface_name = TRY(NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier)); + auto registers_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); + return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) RTL8139NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), move(interface_name)))); } UNMAP_AFTER_INIT RTL8139NetworkAdapter::RTL8139NetworkAdapter(PCI::Address address, u8 irq, NonnullOwnPtr<IOWindow> registers_io_window, NonnullOwnPtr<KString> interface_name) diff --git a/Kernel/Net/Realtek/RTL8139NetworkAdapter.h b/Kernel/Net/Realtek/RTL8139NetworkAdapter.h index 72d82f08af..6d95c85785 100644 --- a/Kernel/Net/Realtek/RTL8139NetworkAdapter.h +++ b/Kernel/Net/Realtek/RTL8139NetworkAdapter.h @@ -22,7 +22,7 @@ class RTL8139NetworkAdapter final : public NetworkAdapter , public PCI::Device , public IRQHandler { public: - static LockRefPtr<RTL8139NetworkAdapter> try_to_initialize(PCI::DeviceIdentifier const&); + static ErrorOr<LockRefPtr<RTL8139NetworkAdapter>> try_to_initialize(PCI::DeviceIdentifier const&); virtual ~RTL8139NetworkAdapter() override; diff --git a/Kernel/Net/Realtek/RTL8168NetworkAdapter.cpp b/Kernel/Net/Realtek/RTL8168NetworkAdapter.cpp index 85ce6b437b..d61d0f629d 100644 --- a/Kernel/Net/Realtek/RTL8168NetworkAdapter.cpp +++ b/Kernel/Net/Realtek/RTL8168NetworkAdapter.cpp @@ -182,19 +182,16 @@ namespace Kernel { #define TX_BUFFER_SIZE 0x1FF8 #define RX_BUFFER_SIZE 0x1FF8 // FIXME: this should be increased (0x3FFF) -UNMAP_AFTER_INIT LockRefPtr<RTL8168NetworkAdapter> RTL8168NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT ErrorOr<LockRefPtr<RTL8168NetworkAdapter>> RTL8168NetworkAdapter::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) { if (pci_device_identifier.hardware_id().vendor_id != PCI::VendorID::Realtek) - return {}; + return nullptr; if (pci_device_identifier.hardware_id().device_id != 0x8168) - return {}; + return nullptr; u8 irq = pci_device_identifier.interrupt_line().value(); - // FIXME: Better propagate errors here - auto interface_name_or_error = NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier); - if (interface_name_or_error.is_error()) - return {}; - auto registers_io_window = MUST(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); - return adopt_lock_ref_if_nonnull(new (nothrow) RTL8168NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), interface_name_or_error.release_value())); + auto interface_name = TRY(NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier)); + auto registers_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); + return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) RTL8168NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), move(interface_name)))); } bool RTL8168NetworkAdapter::determine_supported_version() const diff --git a/Kernel/Net/Realtek/RTL8168NetworkAdapter.h b/Kernel/Net/Realtek/RTL8168NetworkAdapter.h index a0ca643c2b..b075c25aee 100644 --- a/Kernel/Net/Realtek/RTL8168NetworkAdapter.h +++ b/Kernel/Net/Realtek/RTL8168NetworkAdapter.h @@ -22,7 +22,7 @@ class RTL8168NetworkAdapter final : public NetworkAdapter , public PCI::Device , public IRQHandler { public: - static LockRefPtr<RTL8168NetworkAdapter> try_to_initialize(PCI::DeviceIdentifier const&); + static ErrorOr<LockRefPtr<RTL8168NetworkAdapter>> try_to_initialize(PCI::DeviceIdentifier const&); virtual ~RTL8168NetworkAdapter() override; |