Kernel: Fix PCI bridge enumeration

The enumeration code is already enumerating all buses, recursively
enumerating bridges (which are buses) makes devices on bridges being
enumerated multiple times. Also, the PCI code was incorrectly mixing up
terminology; let's settle down on bus, device and function because ever
since PCIe came along "slots" isn't really a thing anymore.

6 files changed, 42 insertions, 42 deletions

diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp
index 7aa5d4deed..f09b0fde2e 100644
--- a/Kernel/FileSystem/ProcFS.cpp
+++ b/Kernel/FileSystem/ProcFS.cpp
@@ -363,7 +363,7 @@ static bool procfs$pci(InodeIdentifier, KBufferBuilder& builder)
         auto obj = array.add_object();
         obj.add("seg", address.seg());
         obj.add("bus", address.bus());
-        obj.add("slot", address.slot());
+        obj.add("slot", address.device());
         obj.add("function", address.function());
         obj.add("vendor_id", id.vendor_id);
         obj.add("device_id", id.device_id);
diff --git a/Kernel/PCI/Access.cpp b/Kernel/PCI/Access.cpp
index 4bbe031265..197f00907b 100644
--- a/Kernel/PCI/Access.cpp
+++ b/Kernel/PCI/Access.cpp
@@ -64,7 +64,7 @@ PhysicalID Access::get_physical_id(Address address) const
     for (auto physical_id : m_physical_ids) {
         if (physical_id.address().seg() == address.seg()
             && physical_id.address().bus() == address.bus()
-            && physical_id.address().slot() == address.slot()
+            && physical_id.address().device() == address.device()
             && physical_id.address().function() == address.function()) {
             return physical_id;
         }
@@ -99,43 +99,43 @@ u16 Access::early_read_type(Address address)
     return (early_read8_field(address, PCI_CLASS) << 8u) | early_read8_field(address, PCI_SUBCLASS);
 }
 
-void Access::enumerate_functions(int type, u8 bus, u8 slot, u8 function, Function<void(Address, ID)>& callback)
+void Access::enumerate_functions(int type, u8 bus, u8 device, u8 function, Function<void(Address, ID)>& callback, bool recursive)
 {
-    dbgln<PCI_DEBUG>("PCI: Enumerating function type={}, bus={}, slot={}, function={}", type, bus, slot, function);
-    Address address(0, bus, slot, function);
+    dbgln<PCI_DEBUG>("PCI: Enumerating function type={}, bus={}, device={}, function={}", type, bus, device, function);
+    Address address(0, bus, device, function);
     if (type == -1 || type == early_read_type(address))
         callback(address, { early_read16_field(address, PCI_VENDOR_ID), early_read16_field(address, PCI_DEVICE_ID) });
-    if (early_read_type(address) == PCI_TYPE_BRIDGE) {
+    if (early_read_type(address) == PCI_TYPE_BRIDGE && recursive) {
         u8 secondary_bus = early_read8_field(address, PCI_SECONDARY_BUS);
 #if PCI_DEBUG
         klog() << "PCI: Found secondary bus: " << secondary_bus;
 #endif
         ASSERT(secondary_bus != bus);
-        enumerate_bus(type, secondary_bus, callback);
+        enumerate_bus(type, secondary_bus, callback, recursive);
     }
 }
 
-void Access::enumerate_slot(int type, u8 bus, u8 slot, Function<void(Address, ID)>& callback)
+void Access::enumerate_device(int type, u8 bus, u8 device, Function<void(Address, ID)>& callback, bool recursive)
 {
-    dbgln<PCI_DEBUG>("PCI: Enumerating slot type={}, bus={}, slot={}", type, bus, slot);
-    Address address(0, bus, slot, 0);
+    dbgln<PCI_DEBUG>("PCI: Enumerating device type={}, bus={}, device={}", type, bus, device);
+    Address address(0, bus, device, 0);
     if (early_read16_field(address, PCI_VENDOR_ID) == PCI_NONE)
         return;
-    enumerate_functions(type, bus, slot, 0, callback);
+    enumerate_functions(type, bus, device, 0, callback, recursive);
     if (!(early_read8_field(address, PCI_HEADER_TYPE) & 0x80))
         return;
     for (u8 function = 1; function < 8; ++function) {
-        Address address(0, bus, slot, function);
+        Address address(0, bus, device, function);
         if (early_read16_field(address, PCI_VENDOR_ID) != PCI_NONE)
-            enumerate_functions(type, bus, slot, function, callback);
+            enumerate_functions(type, bus, device, function, callback, recursive);
     }
 }
 
-void Access::enumerate_bus(int type, u8 bus, Function<void(Address, ID)>& callback)
+void Access::enumerate_bus(int type, u8 bus, Function<void(Address, ID)>& callback, bool recursive)
 {
     dbgln<PCI_DEBUG>("PCI: Enumerating bus type={}, bus={}", type, bus);
-    for (u8 slot = 0; slot < 32; ++slot)
-        enumerate_slot(type, bus, slot, callback);
+    for (u8 device = 0; device < 32; ++device)
+        enumerate_device(type, bus, device, callback, recursive);
 }
 
 void Access::enumerate(Function<void(Address, ID)>& callback) const
diff --git a/Kernel/PCI/Access.h b/Kernel/PCI/Access.h
index 2b53ceaf59..ee42c8a057 100644
--- a/Kernel/PCI/Access.h
+++ b/Kernel/PCI/Access.h
@@ -41,9 +41,9 @@ public:
 
     void enumerate(Function<void(Address, ID)>&) const;
 
-    void enumerate_bus(int type, u8 bus, Function<void(Address, ID)>&);
-    void enumerate_functions(int type, u8 bus, u8 slot, u8 function, Function<void(Address, ID)>& callback);
-    void enumerate_slot(int type, u8 bus, u8 slot, Function<void(Address, ID)>& callback);
+    void enumerate_bus(int type, u8 bus, Function<void(Address, ID)>&, bool recursive);
+    void enumerate_functions(int type, u8 bus, u8 device, u8 function, Function<void(Address, ID)>& callback, bool recursive);
+    void enumerate_device(int type, u8 bus, u8 device, Function<void(Address, ID)>& callback, bool recursive);
 
     static Access& the();
     static bool is_initialized();
diff --git a/Kernel/PCI/Definitions.h b/Kernel/PCI/Definitions.h
index 60bb13d868..98cc7a6715 100644
--- a/Kernel/PCI/Definitions.h
+++ b/Kernel/PCI/Definitions.h
@@ -96,14 +96,14 @@ public:
     Address(u16 seg)
         : m_seg(seg)
         , m_bus(0)
-        , m_slot(0)
+        , m_device(0)
         , m_function(0)
     {
     }
-    Address(u16 seg, u8 bus, u8 slot, u8 function)
+    Address(u16 seg, u8 bus, u8 device, u8 function)
         : m_seg(seg)
         , m_bus(bus)
-        , m_slot(slot)
+        , m_device(device)
         , m_function(function)
     {
     }
@@ -111,12 +111,12 @@ public:
     Address(const Address& address)
         : m_seg(address.seg())
         , m_bus(address.bus())
-        , m_slot(address.slot())
+        , m_device(address.device())
         , m_function(address.function())
     {
     }
 
-    bool is_null() const { return !m_bus && !m_slot && !m_function; }
+    bool is_null() const { return !m_bus && !m_device && !m_function; }
     operator bool() const { return !is_null(); }
 
     // Disable default implementations that would use surprising integer promotion.
@@ -128,24 +128,24 @@ public:
 
     u16 seg() const { return m_seg; }
     u8 bus() const { return m_bus; }
-    u8 slot() const { return m_slot; }
+    u8 device() const { return m_device; }
     u8 function() const { return m_function; }
 
     u32 io_address_for_field(u8 field) const
     {
-        return 0x80000000u | (m_bus << 16u) | (m_slot << 11u) | (m_function << 8u) | (field & 0xfc);
+        return 0x80000000u | (m_bus << 16u) | (m_device << 11u) | (m_function << 8u) | (field & 0xfc);
     }
 
 protected:
     u32 m_seg { 0 };
     u8 m_bus { 0 };
-    u8 m_slot { 0 };
+    u8 m_device { 0 };
     u8 m_function { 0 };
 };
 
 inline const LogStream& operator<<(const LogStream& stream, const Address value)
 {
-    return stream << "PCI [" << String::formatted("{:04x}:{:02x}:{:02x}:{:02x}", value.seg(), value.bus(), value.slot(), value.function()) << "]";
+    return stream << "PCI [" << String::formatted("{:04x}:{:02x}:{:02x}:{:02x}", value.seg(), value.bus(), value.device(), value.function()) << "]";
 }
 
 struct ChangeableAddress : public Address {
@@ -157,17 +157,17 @@ struct ChangeableAddress : public Address {
         : Address(seg)
     {
     }
-    ChangeableAddress(u16 seg, u8 bus, u8 slot, u8 function)
-        : Address(seg, bus, slot, function)
+    ChangeableAddress(u16 seg, u8 bus, u8 device, u8 function)
+        : Address(seg, bus, device, function)
     {
     }
     void set_seg(u16 seg) { m_seg = seg; }
     void set_bus(u8 bus) { m_bus = bus; }
-    void set_slot(u8 slot) { m_slot = slot; }
+    void set_device(u8 device) { m_device = device; }
     void set_function(u8 function) { m_function = function; }
     bool operator==(const Address& address)
     {
-        if (m_seg == address.seg() && m_bus == address.bus() && m_slot == address.slot() && m_function == address.function())
+        if (m_seg == address.seg() && m_bus == address.bus() && m_device == address.device() && m_function == address.function())
             return true;
         else
             return false;
@@ -176,7 +176,7 @@ struct ChangeableAddress : public Address {
     {
         set_seg(address.seg());
         set_bus(address.bus());
-        set_slot(address.slot());
+        set_device(address.device());
         set_function(address.function());
         return *this;
     }
@@ -252,6 +252,6 @@ struct AK::Formatter<Kernel::PCI::Address> : Formatter<FormatString> {
     {
         return Formatter<FormatString>::format(
             builder,
-            "PCI [{:04x}:{:02x}:{:02x}:{:02x}]", value.seg(), value.bus(), value.slot(), value.function());
+            "PCI [{:04x}:{:02x}:{:02x}:{:02x}]", value.seg(), value.bus(), value.device(), value.function());
     }
 };
diff --git a/Kernel/PCI/IOAccess.cpp b/Kernel/PCI/IOAccess.cpp
index 8ca61245b7..46b5f66f97 100644
--- a/Kernel/PCI/IOAccess.cpp
+++ b/Kernel/PCI/IOAccess.cpp
@@ -91,15 +91,15 @@ void IOAccess::enumerate_hardware(Function<void(Address, ID)> callback)
 #endif
     // Single PCI host controller.
     if ((read8_field(Address(), PCI_HEADER_TYPE) & 0x80) == 0) {
-        enumerate_bus(-1, 0, callback);
+        enumerate_bus(-1, 0, callback, true);
         return;
     }
 
     // Multiple PCI host controllers.
-    for (u8 function = 0; function < 8; ++function) {
-        if (read16_field(Address(0, 0, 0, function), PCI_VENDOR_ID) == PCI_NONE)
+    for (int bus = 0; bus < 256; ++bus) {
+        if (read16_field(Address(0, 0, 0, bus), PCI_VENDOR_ID) == PCI_NONE)
             break;
-        enumerate_bus(-1, function, callback);
+        enumerate_bus(-1, bus, callback, false);
     }
 }
 
diff --git a/Kernel/PCI/MMIOAccess.cpp b/Kernel/PCI/MMIOAccess.cpp
index 56831b5c21..1d505653eb 100644
--- a/Kernel/PCI/MMIOAccess.cpp
+++ b/Kernel/PCI/MMIOAccess.cpp
@@ -55,7 +55,7 @@ DeviceConfigurationSpaceMapping::DeviceConfigurationSpaceMapping(Address device_
 {
     PhysicalAddress segment_lower_addr = mmio_segment.get_paddr();
     PhysicalAddress device_physical_mmio_space = segment_lower_addr.offset(
-        PCI_MMIO_CONFIG_SPACE_SIZE * m_device_address.function() + (PCI_MMIO_CONFIG_SPACE_SIZE * PCI_MAX_FUNCTIONS_PER_DEVICE) * m_device_address.slot() + (PCI_MMIO_CONFIG_SPACE_SIZE * PCI_MAX_FUNCTIONS_PER_DEVICE * PCI_MAX_DEVICES_PER_BUS) * (m_device_address.bus() - mmio_segment.get_start_bus()));
+        PCI_MMIO_CONFIG_SPACE_SIZE * m_device_address.function() + (PCI_MMIO_CONFIG_SPACE_SIZE * PCI_MAX_FUNCTIONS_PER_DEVICE) * m_device_address.device() + (PCI_MMIO_CONFIG_SPACE_SIZE * PCI_MAX_FUNCTIONS_PER_DEVICE * PCI_MAX_DEVICES_PER_BUS) * (m_device_address.bus() - mmio_segment.get_start_bus()));
     m_mapped_region->physical_page_slot(0) = PhysicalPage::create(device_physical_mmio_space, false, false);
     m_mapped_region->remap();
 }
@@ -139,7 +139,7 @@ Optional<VirtualAddress> MMIOAccess::get_device_configuration_space(Address addr
         dbgln<PCI_DEBUG>("PCI Device Configuration Space Mapping: Check if {} was requested", checked_address);
         if (address.seg() == checked_address.seg()
             && address.bus() == checked_address.bus()
-            && address.slot() == checked_address.slot()
+            && address.device() == checked_address.device()
             && address.function() == checked_address.function()) {
             dbgln<PCI_DEBUG>("PCI Device Configuration Space Mapping: Found {}", checked_address);
             return mapping.vaddr();
@@ -202,7 +202,7 @@ void MMIOAccess::enumerate_hardware(Function<void(Address, ID)> callback)
         dbgln<PCI_DEBUG>("PCI: Enumerating Memory mapped IO segment {}", seg);
         // Single PCI host controller.
         if ((early_read8_field(Address(seg), PCI_HEADER_TYPE) & 0x80) == 0) {
-            enumerate_bus(-1, 0, callback);
+            enumerate_bus(-1, 0, callback, true);
             return;
         }
 
@@ -210,7 +210,7 @@ void MMIOAccess::enumerate_hardware(Function<void(Address, ID)> callback)
         for (u8 function = 0; function < 8; ++function) {
             if (early_read16_field(Address(seg, 0, 0, function), PCI_VENDOR_ID) == PCI_NONE)
                 break;
-            enumerate_bus(-1, function, callback);
+            enumerate_bus(-1, function, callback, false);
         }
     }
 }