Kernel/Net: Integrate the E1000E driver into the E1000 driver

This removes quite a bit of duplicate code.
The only real remaining difference between the E1000 driver and its
subclassed E1000E driver is that the E1000E driver supports MDIO
access.

Keeping track of hardware differences in a m_hardware_features member
seems better than adding new subclasses.

Documentation/BareMetalInstallation.md

@@ -11,7 +11,7 @@ A minimum of 256 MB RAM and a x86-64 CPU are required.
 
 You must be willing to wipe your disk's contents to allow for writing the Serenity image so be sure to back up any important data on your disk first! Serenity uses the GRUB2 bootloader so it should be possible to multiboot it with any other OS that can be booted from GRUB2 post-installation.
 
-At present there is no real GPU support so don't expect OpenGL, Vulkan nor accelerated video playback and encoding support. Serenity currently relies upon the multiboot or EFI GOP framebuffer on real hardware. There is no WiFi support and the network card chipsets that are currently supported: Intel e1000, Intel e1000e and Realtek 8168. The e1000 driver has only been tested with qemu and VirtualBox although it may work with NICs such as those using the Intel 82545XX, 82540XX, 82546XX or similar chipsets. Supported sound cards are Intel AC'97 and Intel HDA PCI devices.
+At present there is no real GPU support so don't expect OpenGL, Vulkan nor accelerated video playback and encoding support. Serenity currently relies upon the multiboot or EFI GOP framebuffer on real hardware. There is no WiFi support and the network card chipsets that are currently supported: Intel e1000 and Realtek 8168. The e1000 driver has only been tested with qemu and VirtualBox although it may work with NICs such as those using the Intel 82545XX, 82540XX, 82546XX or similar chipsets. Supported sound cards are Intel AC'97 and Intel HDA PCI devices.
 
 ## Creating a Serenity GRUB disk image
 

Kernel/CMakeLists.txt

@@ -285,7 +285,6 @@ set(KERNEL_SOURCES
     Library/KLexicalPath.cpp
     Library/KString.cpp
     Library/UserOrKernelBuffer.cpp
-    Net/Intel/E1000ENetworkAdapter.cpp
     Net/Intel/E1000NetworkAdapter.cpp
     Net/Realtek/RTL8168NetworkAdapter.cpp
     Net/VirtIO/VirtIONetworkAdapter.cpp

Kernel/Net/Intel/E1000ENetworkAdapter.cpp

@@ -1,144 +0,0 @@
-/*
- * Copyright (c) 2018-2021, Andreas Kling <[email protected]>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <AK/MACAddress.h>
-#include <Kernel/Bus/PCI/API.h>
-#include <Kernel/Bus/PCI/IDs.h>
-#include <Kernel/Net/Intel/E1000ENetworkAdapter.h>
-#include <Kernel/Net/NetworkingManagement.h>
-#include <Kernel/Sections.h>
-#include <Kernel/Tasks/Process.h>
-
-namespace Kernel {
-
-#define REG_EECD 0x0010
-#define REG_EEPROM 0x0014
-#define REG_MDIC 0x0020
-
-// EECD Register
-
-#define EECD_PRES 0x100
-
-// MIDC Register
-
-#define REG_MDIC_DATA_MASK (0xffff << 0)
-#define REG_MDIC_DATA_OFFSET 0
-#define REG_MDIC_REGADD_OFFSET 16
-#define REG_MDIC_R (1 << 28)
-#define REG_MDIC_OP_WRITE (0b01 << 26)
-#define REG_MDIC_OP_READ (0b10 << 26)
-#define REG_MDIC_PHYADD_OFFSET 21
-
-#define GIGABIT_PHY_ID 1
-
-static bool is_valid_device_id(u16 device_id)
-{
-    switch (device_id) {
-    case 0x10D3: // 82574L
-        return true;
-    default:
-        return false;
-    }
-}
-
-UNMAP_AFTER_INIT ErrorOr<bool> E1000ENetworkAdapter::probe(PCI::DeviceIdentifier const& pci_device_identifier)
-{
-    if (pci_device_identifier.hardware_id().vendor_id != PCI::VendorID::Intel)
-        return false;
-    return is_valid_device_id(pci_device_identifier.hardware_id().device_id);
-}
-
-UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<NetworkAdapter>> E1000ENetworkAdapter::create(PCI::DeviceIdentifier const& pci_device_identifier)
-{
-    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 rx_buffer_region = TRY(MM.allocate_contiguous_kernel_region(rx_buffer_size * number_of_rx_descriptors, "E1000 RX buffers"sv, Memory::Region::Access::ReadWrite));
-    auto tx_buffer_region = MM.allocate_contiguous_kernel_region(tx_buffer_size * number_of_tx_descriptors, "E1000 TX buffers"sv, Memory::Region::Access::ReadWrite).release_value();
-    auto rx_descriptors_region = TRY(Memory::allocate_dma_region_as_typed_array<RxDescriptor volatile>(number_of_rx_descriptors, "E1000 RX Descriptors"sv, Memory::Region::Access::ReadWrite));
-    auto tx_descriptors_region = TRY(Memory::allocate_dma_region_as_typed_array<TxDescriptor volatile>(number_of_tx_descriptors, "E1000 TX Descriptors"sv, Memory::Region::Access::ReadWrite));
-
-    return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) E1000ENetworkAdapter(interface_name.representable_view(),
-        pci_device_identifier,
-        move(registers_io_window),
-        move(rx_buffer_region),
-        move(tx_buffer_region),
-        move(rx_descriptors_region),
-        move(tx_descriptors_region))));
-}
-
-UNMAP_AFTER_INIT ErrorOr<void> E1000ENetworkAdapter::initialize(Badge<NetworkingManagement>)
-{
-    dmesgln("E1000e: Found @ {}", device_identifier().address());
-    enable_bus_mastering(device_identifier());
-
-    dmesgln("E1000e: IO base: {}", m_registers_io_window);
-    read_mac_address();
-    auto const& mac = mac_address();
-    dmesgln("E1000e: MAC address: {}", mac.to_string());
-
-    m_mdio_handling_process = TRY(spawn_mdio_handling_task(GIGABIT_PHY_ID));
-
-    initialize_rx_descriptors();
-    initialize_tx_descriptors();
-
-    setup_link();
-    TRY(setup_interrupts());
-    autoconfigure_link_local_ipv6();
-    return {};
-}
-
-UNMAP_AFTER_INIT E1000ENetworkAdapter::E1000ENetworkAdapter(StringView interface_name,
-    PCI::DeviceIdentifier const& device_identifier,
-    NonnullOwnPtr<IOWindow> registers_io_window, NonnullOwnPtr<Memory::Region> rx_buffer_region,
-    NonnullOwnPtr<Memory::Region> tx_buffer_region, Memory::TypedMapping<RxDescriptor volatile[]> rx_descriptors,
-    Memory::TypedMapping<TxDescriptor volatile[]> tx_descriptors)
-    : E1000NetworkAdapter(interface_name, device_identifier, move(registers_io_window),
-          move(rx_buffer_region),
-          move(tx_buffer_region),
-          move(rx_descriptors),
-          move(tx_descriptors))
-{
-}
-
-UNMAP_AFTER_INIT E1000ENetworkAdapter::~E1000ENetworkAdapter()
-{
-    if (m_mdio_handling_process) {
-        m_mdio_handling_process->die();
-        // Block until all threads exited to prevent UAF
-        ErrorOr<siginfo_t> result = siginfo_t {};
-        (void)Thread::current()->block<Thread::WaitBlocker>({}, WEXITED, m_mdio_handling_process.release_nonnull(), result);
-    }
-}
-
-u16 E1000ENetworkAdapter::read_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address)
-{
-    // FIXME: Newer controllers (like the I211) don't have a PHYADD field in the MDIC register.
-    //        They instead have it in a separate register.
-    out32(REG_MDIC, REG_MDIC_OP_READ | (to_underlying(address) << REG_MDIC_REGADD_OFFSET) | (phy_id << REG_MDIC_PHYADD_OFFSET));
-
-    for (;;) {
-        u32 mdic = in32(REG_MDIC);
-        if ((mdic & REG_MDIC_R) == 0) {
-            Processor::wait_check();
-            continue;
-        }
-
-        return (mdic & REG_MDIC_DATA_MASK) >> REG_MDIC_DATA_OFFSET;
-    }
-}
-
-void E1000ENetworkAdapter::write_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address, u16 value)
-{
-    // FIXME: Newer controllers (like the I211) don't have a PHYADD field in the MDIC register.
-    //        They instead have it in a separate register.
-    out32(REG_MDIC, REG_MDIC_OP_WRITE | (to_underlying(address) << REG_MDIC_REGADD_OFFSET) | (value << REG_MDIC_DATA_OFFSET) | (phy_id << REG_MDIC_PHYADD_OFFSET));
-
-    while ((in32(REG_MDIC) & REG_MDIC_R) == 0)
-        Processor::wait_check();
-}
-
-}

Kernel/Net/Intel/E1000ENetworkAdapter.h

@@ -1,46 +0,0 @@
-/*
- * Copyright (c) 2021, Liav A. <[email protected]>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <AK/OwnPtr.h>
-#include <Kernel/Bus/PCI/Access.h>
-#include <Kernel/Bus/PCI/Device.h>
-#include <Kernel/Interrupts/IRQHandler.h>
-#include <Kernel/Library/IOWindow.h>
-#include <Kernel/Net/Intel/E1000NetworkAdapter.h>
-#include <Kernel/Net/MDIO.h>
-#include <Kernel/Net/NetworkAdapter.h>
-#include <Kernel/Security/Random.h>
-
-namespace Kernel {
-
-class E1000ENetworkAdapter final
-    : public E1000NetworkAdapter
-    , public MDIO::Clause22::Interface {
-public:
-    static ErrorOr<bool> probe(PCI::DeviceIdentifier const&);
-    static ErrorOr<NonnullRefPtr<NetworkAdapter>> create(PCI::DeviceIdentifier const&);
-    virtual ErrorOr<void> initialize(Badge<NetworkingManagement>) override;
-
-    virtual ~E1000ENetworkAdapter() override;
-
-protected:
-    // ^MDIO::Clause22::Interface
-    virtual u16 read_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address) override;
-    virtual void write_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address, u16 value) override;
-
-private:
-    E1000ENetworkAdapter(StringView interface_name, PCI::DeviceIdentifier const&,
-        NonnullOwnPtr<IOWindow> registers_io_window, NonnullOwnPtr<Memory::Region> rx_buffer_region,
-        NonnullOwnPtr<Memory::Region> tx_buffer_region, Memory::TypedMapping<RxDescriptor volatile[]> rx_descriptors,
-        Memory::TypedMapping<TxDescriptor volatile[]> tx_descriptors);
-
-    virtual StringView class_name() const override { return "E1000ENetworkAdapter"sv; }
-
-    RefPtr<Process> m_mdio_handling_process;
-};
-}

Kernel/Net/Intel/E1000NetworkAdapter.cpp

@@ -18,6 +18,7 @@ namespace Kernel {
 #define REG_STATUS 0x0008
 #define REG_EEPROM 0x0014
 #define REG_CTRL_EXT 0x0018
+#define REG_MDIC 0x0020
 #define REG_INTERRUPT_CAUSE_READ 0x00C0
 #define REG_INTERRUPT_RATE 0x00C4
 #define REG_INTERRUPT_MASK_SET 0x00D0
@@ -123,13 +124,22 @@ namespace Kernel {
 #define INTERRUPT_TXD_LOW (1 << 15)
 #define INTERRUPT_SRPD (1 << 16)
 
+// MDIC Register
+
+#define MDIC_DATA_MASK (0xffff << 0)
+#define MDIC_DATA_OFFSET 0
+#define MDIC_REGADD_OFFSET 16
+#define MDIC_R (1 << 28)
+#define MDIC_OP_WRITE (0b01 << 26)
+#define MDIC_OP_READ (0b10 << 26)
+#define MDIC_PHYADD_OFFSET 21
+
+#define GIGABIT_PHY_ID 1
+
 // https://www.intel.com/content/dam/doc/manual/pci-pci-x-family-gbe-controllers-software-dev-manual.pdf Section 5.2
 UNMAP_AFTER_INIT static bool is_valid_device_id(u16 device_id)
 {
-    // FIXME: It would be nice to distinguish which particular device it is.
-    //        Especially since it's needed to determine which registers we can access.
-    //        The reason I haven't done it now is because there's some IDs with multiple devices
-    //        and some devices with multiple IDs.
+    // FIXME: Merge this function with determine_hardware_features().
     switch (device_id) {
     case 0x1019: // 82547EI-A0, 82547EI-A1, 82547EI-B0, 82547GI-B0
     case 0x101A: // 82547EI-B0
@@ -155,6 +165,7 @@ UNMAP_AFTER_INIT static bool is_valid_device_id(u16 device_id)
     case 0x1016: // 82540EP-A
     case 0x100E: // 82540EM-A
     case 0x1015: // 82540EM-A
+    case 0x10D3: // 82574L
         return true;
     default:
         return false;
@@ -199,6 +210,9 @@ UNMAP_AFTER_INIT ErrorOr<void> E1000NetworkAdapter::initialize(Badge<NetworkingM
     auto const& mac = mac_address();
     dmesgln_pci(*this, "MAC address: {}", mac.to_string());
 
+    if (has_flag(m_hardware_features, HardwareFeatures::MDIOAccess))
+        m_mdio_handling_process = TRY(spawn_mdio_handling_task(GIGABIT_PHY_ID));
+
     initialize_rx_descriptors();
     initialize_tx_descriptors();
 
@@ -217,6 +231,17 @@ UNMAP_AFTER_INIT void E1000NetworkAdapter::setup_link()
     out32(REG_CTRL, flags | ECTRL_SLU);
 }
 
+E1000NetworkAdapter::HardwareFeatures E1000NetworkAdapter::determine_hardware_features()
+{
+    auto device_id = device_identifier().hardware_id().device_id;
+    switch (device_id) {
+    case 0x10D3: // 82574L
+        return HardwareFeatures::MDIOAccess;
+    default:
+        return HardwareFeatures::None;
+    }
+}
+
 UNMAP_AFTER_INIT ErrorOr<void> E1000NetworkAdapter::setup_interrupts()
 {
     out32(REG_INTERRUPT_RATE, 6000); // Interrupt rate of 1.536 milliseconds
@@ -284,6 +309,37 @@ bool E1000NetworkAdapter::handle_interrupt()
     return true;
 }
 
+u16 E1000NetworkAdapter::read_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address)
+{
+    VERIFY(has_flag(m_hardware_features, HardwareFeatures::MDIOAccess));
+
+    // FIXME: Newer controllers (like the I211) don't have a PHYADD field in the MDIC register.
+    //        They instead have it in a separate register.
+    out32(REG_MDIC, MDIC_OP_READ | (to_underlying(address) << MDIC_REGADD_OFFSET) | (phy_id << MDIC_PHYADD_OFFSET));
+
+    for (;;) {
+        u32 mdic = in32(REG_MDIC);
+        if ((mdic & MDIC_R) == 0) {
+            Processor::wait_check();
+            continue;
+        }
+
+        return (mdic & MDIC_DATA_MASK) >> MDIC_DATA_OFFSET;
+    }
+}
+
+void E1000NetworkAdapter::write_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address, u16 value)
+{
+    VERIFY(has_flag(m_hardware_features, HardwareFeatures::MDIOAccess));
+
+    // FIXME: Newer controllers (like the I211) don't have a PHYADD field in the MDIC register.
+    //        They instead have it in a separate register.
+    out32(REG_MDIC, MDIC_OP_WRITE | (to_underlying(address) << MDIC_REGADD_OFFSET) | (value << MDIC_DATA_OFFSET) | (phy_id << MDIC_PHYADD_OFFSET));
+
+    while ((in32(REG_MDIC) & MDIC_R) == 0)
+        Processor::wait_check();
+}
+
 UNMAP_AFTER_INIT void E1000NetworkAdapter::read_mac_address()
 {
     auto ral0 = in32(REG_RAL0);

Kernel/Net/Intel/E1000NetworkAdapter.h

@@ -12,13 +12,17 @@
 #include <Kernel/Bus/PCI/Device.h>
 #include <Kernel/Interrupts/PCIIRQHandler.h>
 #include <Kernel/Library/IOWindow.h>
+#include <Kernel/Net/MDIO.h>
 #include <Kernel/Net/NetworkAdapter.h>
 #include <Kernel/Security/Random.h>
+#include <Kernel/Tasks/Process.h>
 #include <Kernel/Tasks/WaitQueue.h>
 
 namespace Kernel {
 
-class E1000NetworkAdapter : public NetworkAdapter
+class E1000NetworkAdapter
+    : public NetworkAdapter
+    , public MDIO::Clause22::Interface
     , public PCI::Device {
 public:
     static ErrorOr<bool> probe(PCI::DeviceIdentifier const&);
@@ -35,7 +39,7 @@ public:
     virtual StringView device_name() const override { return "E1000"sv; }
     virtual Type adapter_type() const override { return Type::Ethernet; }
 
-protected:
+private:
     static constexpr size_t rx_buffer_size = 8192;
     static constexpr size_t tx_buffer_size = 8192;
 
@@ -60,6 +64,15 @@ protected:
     };
     static_assert(AssertSize<TxDescriptor, 16>());
 
+    enum class HardwareFeatures {
+        None = 0u,
+
+        MDIOAccess = 1u << 0,
+    };
+    AK_ENUM_BITWISE_FRIEND_OPERATORS(E1000NetworkAdapter::HardwareFeatures);
+
+    HardwareFeatures determine_hardware_features();
+
     ErrorOr<void> setup_interrupts();
     void setup_link();
 
@@ -99,6 +112,10 @@ protected:
 
     virtual StringView class_name() const override { return "E1000NetworkAdapter"sv; }
 
+    // ^MDIO::Clause22::Interface
+    virtual u16 read_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address) override;
+    virtual void write_phy_register(u8 phy_id, MDIO::Clause22::RegisterAddress address, u16 value) override;
+
     void read_mac_address();
 
     void initialize_rx_descriptors();
@@ -112,6 +129,8 @@ protected:
     static constexpr size_t number_of_rx_descriptors = 256;
     static constexpr size_t number_of_tx_descriptors = 256;
 
+    HardwareFeatures m_hardware_features;
+
     NonnullOwnPtr<IOWindow> m_registers_io_window;
 
     Memory::TypedMapping<RxDescriptor volatile[]> m_rx_descriptors;
@@ -129,5 +148,7 @@ protected:
     WaitQueue m_wait_queue;
 
     OwnPtr<InterruptHandler> m_interrupt_handler;
+    RefPtr<Process> m_mdio_handling_process;
 };
+
 }

Kernel/Net/NetworkingManagement.cpp

@@ -9,7 +9,6 @@
 #include <Kernel/Bus/PCI/API.h>
 #include <Kernel/Library/KString.h>
 #include <Kernel/Memory/AnonymousVMObject.h>
-#include <Kernel/Net/Intel/E1000ENetworkAdapter.h>
 #include <Kernel/Net/Intel/E1000NetworkAdapter.h>
 #include <Kernel/Net/LoopbackAdapter.h>
 #include <Kernel/Net/NetworkingManagement.h>
@@ -112,7 +111,6 @@ struct PCINetworkDriverInitializer {
 static constexpr PCINetworkDriverInitializer s_initializers[] = {
     { RTL8168NetworkAdapter::probe, RTL8168NetworkAdapter::create },
     { E1000NetworkAdapter::probe, E1000NetworkAdapter::create },
-    { E1000ENetworkAdapter::probe, E1000ENetworkAdapter::create },
     { VirtIONetworkAdapter::probe, VirtIONetworkAdapter::create },
 };
 

Meta/gn/secondary/Kernel/BUILD.gn

@@ -696,7 +696,6 @@ executable("Kernel_bin") {
     "Memory/VMObject.cpp",
     "Memory/VirtualRange.cpp",
     "Net/IP/Socket.cpp",
-    "Net/Intel/E1000ENetworkAdapter.cpp",
     "Net/Intel/E1000NetworkAdapter.cpp",
     "Net/LocalSocket.cpp",
     "Net/LoopbackAdapter.cpp",