Kernel/PCI: Add small access spinlock to each HostController

Prepare to remove biglock on PCI::Access in a future commit, so we can
ensure we only lock a spinlock on a precise PCI HostController if needed
instead of the entire subsystem.

Kernel/Arch/x86_64/PCI/Controller/HostBridge.cpp

@@ -27,34 +27,44 @@ static u32 io_address_for_pci_field(BusNumber bus, DeviceNumber device, Function
     return 0x80000000u | (bus.value() << 16u) | (device.value() << 11u) | (function.value() << 8u) | (field & 0xfc);
 }
 
-void HostBridge::write8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u8 value)
+void HostBridge::write8_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u8 value)
 {
+    VERIFY(m_access_lock.is_locked());
     IO::out32(PCI::address_port, io_address_for_pci_field(bus, device, function, field));
     IO::out8(PCI::value_port + (field & 3), value);
 }
-void HostBridge::write16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u16 value)
+
+void HostBridge::write16_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u16 value)
 {
+    VERIFY(m_access_lock.is_locked());
     IO::out32(PCI::address_port, io_address_for_pci_field(bus, device, function, field));
     IO::out16(PCI::value_port + (field & 2), value);
 }
-void HostBridge::write32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u32 value)
+
+void HostBridge::write32_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u32 value)
 {
+    VERIFY(m_access_lock.is_locked());
     IO::out32(PCI::address_port, io_address_for_pci_field(bus, device, function, field));
     IO::out32(PCI::value_port, value);
 }
 
-u8 HostBridge::read8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+u8 HostBridge::read8_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
+    VERIFY(m_access_lock.is_locked());
     IO::out32(PCI::address_port, io_address_for_pci_field(bus, device, function, field));
     return IO::in8(PCI::value_port + (field & 3));
 }
-u16 HostBridge::read16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+
+u16 HostBridge::read16_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
+    VERIFY(m_access_lock.is_locked());
     IO::out32(PCI::address_port, io_address_for_pci_field(bus, device, function, field));
     return IO::in16(PCI::value_port + (field & 2));
 }
-u32 HostBridge::read32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+
+u32 HostBridge::read32_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
+    VERIFY(m_access_lock.is_locked());
     IO::out32(PCI::address_port, io_address_for_pci_field(bus, device, function, field));
     return IO::in32(PCI::value_port);
 }

Kernel/Arch/x86_64/PCI/Controller/HostBridge.h

@@ -19,15 +19,15 @@ class HostBridge : public HostController {
 public:
     static NonnullOwnPtr<HostBridge> must_create_with_io_access();
 
-    virtual void write8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) override;
-    virtual void write16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) override;
-    virtual void write32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) override;
-
-    virtual u8 read8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-    virtual u16 read16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-    virtual u32 read32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-
 private:
+    virtual void write8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) override;
+    virtual void write16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) override;
+    virtual void write32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) override;
+
+    virtual u8 read8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+    virtual u16 read16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+    virtual u32 read32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+
     explicit HostBridge(PCI::Domain const&);
 };
 

Kernel/Bus/PCI/Controller/HostController.cpp

@@ -19,6 +19,42 @@ HostController::HostController(PCI::Domain const& domain)
 {
 }
 
+void HostController::write8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u8 value)
+{
+    SpinlockLocker locker(m_access_lock);
+    write8_field_locked(bus, device, function, field, value);
+}
+
+void HostController::write16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u16 value)
+{
+    SpinlockLocker locker(m_access_lock);
+    write16_field_locked(bus, device, function, field, value);
+}
+
+void HostController::write32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u32 value)
+{
+    SpinlockLocker locker(m_access_lock);
+    write32_field_locked(bus, device, function, field, value);
+}
+
+u8 HostController::read8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+{
+    SpinlockLocker locker(m_access_lock);
+    return read8_field_locked(bus, device, function, field);
+}
+
+u16 HostController::read16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+{
+    SpinlockLocker locker(m_access_lock);
+    return read16_field_locked(bus, device, function, field);
+}
+
+u32 HostController::read32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+{
+    SpinlockLocker locker(m_access_lock);
+    return read32_field_locked(bus, device, function, field);
+}
+
 UNMAP_AFTER_INIT Optional<u8> HostController::get_capabilities_pointer_for_function(BusNumber bus, DeviceNumber device, FunctionNumber function)
 {
     if (read16_field(bus, device, function, PCI::RegisterOffset::STATUS) & (1 << 4)) {

Kernel/Bus/PCI/Controller/HostController.h

@@ -73,13 +73,13 @@ class HostController {
 public:
     virtual ~HostController() = default;
 
-    virtual void write8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) = 0;
-    virtual void write16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) = 0;
-    virtual void write32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) = 0;
+    void write8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value);
+    void write16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value);
+    void write32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value);
 
-    virtual u8 read8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) = 0;
-    virtual u16 read16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) = 0;
-    virtual u32 read32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) = 0;
+    u8 read8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field);
+    u16 read16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field);
+    u32 read32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field);
 
     u32 domain_number() const { return m_domain.domain_number(); }
 
@@ -102,10 +102,20 @@ private:
     Vector<Capability> get_capabilities_for_function(BusNumber, DeviceNumber, FunctionNumber);
 
 protected:
+    virtual void write8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) = 0;
+    virtual void write16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) = 0;
+    virtual void write32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) = 0;
+
+    virtual u8 read8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) = 0;
+    virtual u16 read16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) = 0;
+    virtual u32 read32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) = 0;
+
     explicit HostController(PCI::Domain const& domain);
 
     const PCI::Domain m_domain;
 
+    Spinlock<LockRank::None> m_access_lock;
+
 private:
     Bitmap m_enumerated_buses;
 };

Kernel/Bus/PCI/Controller/MemoryBackedHostBridge.cpp

@@ -22,50 +22,50 @@ MemoryBackedHostBridge::MemoryBackedHostBridge(PCI::Domain const& domain, Physic
 {
 }
 
-u8 MemoryBackedHostBridge::read8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+u8 MemoryBackedHostBridge::read8_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     VERIFY(field <= 0xfff);
     return *((u8 volatile*)(get_device_configuration_memory_mapped_space(bus, device, function).get() + (field & 0xfff)));
 }
-u16 MemoryBackedHostBridge::read16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+u16 MemoryBackedHostBridge::read16_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     VERIFY(field < 0xfff);
     u16 data = 0;
     ByteReader::load<u16>(get_device_configuration_memory_mapped_space(bus, device, function).offset(field & 0xfff).as_ptr(), data);
     return data;
 }
-u32 MemoryBackedHostBridge::read32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+u32 MemoryBackedHostBridge::read32_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     VERIFY(field <= 0xffc);
     u32 data = 0;
     ByteReader::load<u32>(get_device_configuration_memory_mapped_space(bus, device, function).offset(field & 0xfff).as_ptr(), data);
     return data;
 }
-void MemoryBackedHostBridge::write8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u8 value)
+void MemoryBackedHostBridge::write8_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u8 value)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     VERIFY(field <= 0xfff);
     *((u8 volatile*)(get_device_configuration_memory_mapped_space(bus, device, function).get() + (field & 0xfff))) = value;
 }
-void MemoryBackedHostBridge::write16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u16 value)
+void MemoryBackedHostBridge::write16_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u16 value)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     VERIFY(field < 0xfff);
     ByteReader::store<u16>(get_device_configuration_memory_mapped_space(bus, device, function).offset(field & 0xfff).as_ptr(), value);
 }
-void MemoryBackedHostBridge::write32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u32 value)
+void MemoryBackedHostBridge::write32_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u32 value)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     VERIFY(field <= 0xffc);
     ByteReader::store<u32>(get_device_configuration_memory_mapped_space(bus, device, function).offset(field & 0xfff).as_ptr(), value);
 }
 
 void MemoryBackedHostBridge::map_bus_region(BusNumber bus)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     if (m_mapped_bus == bus && m_mapped_bus_region)
         return;
     auto bus_base_address = determine_memory_mapped_bus_base_address(bus);
@@ -80,7 +80,7 @@ void MemoryBackedHostBridge::map_bus_region(BusNumber bus)
 
 VirtualAddress MemoryBackedHostBridge::get_device_configuration_memory_mapped_space(BusNumber bus, DeviceNumber device, FunctionNumber function)
 {
-    VERIFY(Access::the().access_lock().is_locked());
+    VERIFY(m_access_lock.is_locked());
     map_bus_region(bus);
     return m_mapped_bus_region->vaddr().offset(mmio_device_space_size * function.value() + (mmio_device_space_size * to_underlying(Limits::MaxFunctionsPerDevice)) * device.value());
 }

Kernel/Bus/PCI/Controller/MemoryBackedHostBridge.h

@@ -18,15 +18,15 @@ class MemoryBackedHostBridge : public HostController {
 public:
     static NonnullOwnPtr<MemoryBackedHostBridge> must_create(Domain const&, PhysicalAddress);
 
-    virtual void write8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) override;
-    virtual void write16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) override;
-    virtual void write32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) override;
-
-    virtual u8 read8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-    virtual u16 read16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-    virtual u32 read32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-
 protected:
+    virtual void write8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) override;
+    virtual void write16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) override;
+    virtual void write32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) override;
+
+    virtual u8 read8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+    virtual u16 read16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+    virtual u32 read32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+
     MemoryBackedHostBridge(PCI::Domain const&, PhysicalAddress);
 
     // Memory-mapped access operations

Kernel/Bus/PCI/Controller/VolumeManagementDevice.cpp

@@ -41,42 +41,42 @@ NonnullOwnPtr<VolumeManagementDevice> VolumeManagementDevice::must_create(PCI::D
     return adopt_own_if_nonnull(new (nothrow) VolumeManagementDevice(domain, start_address)).release_nonnull();
 }
 
-void VolumeManagementDevice::write8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u8 value)
+void VolumeManagementDevice::write8_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u8 value)
 {
     SpinlockLocker locker(m_config_lock);
     // Note: We must write then read to ensure completion before returning.
-    MemoryBackedHostBridge::write8_field(bus, device, function, field, value);
-    MemoryBackedHostBridge::read8_field(bus, device, function, field);
+    MemoryBackedHostBridge::write8_field_locked(bus, device, function, field, value);
+    MemoryBackedHostBridge::read8_field_locked(bus, device, function, field);
 }
-void VolumeManagementDevice::write16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u16 value)
+void VolumeManagementDevice::write16_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u16 value)
 {
     SpinlockLocker locker(m_config_lock);
     // Note: We must write then read to ensure completion before returning.
-    MemoryBackedHostBridge::write16_field(bus, device, function, field, value);
-    MemoryBackedHostBridge::read16_field(bus, device, function, field);
+    MemoryBackedHostBridge::write16_field_locked(bus, device, function, field, value);
+    MemoryBackedHostBridge::read16_field_locked(bus, device, function, field);
 }
-void VolumeManagementDevice::write32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u32 value)
+void VolumeManagementDevice::write32_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field, u32 value)
 {
     SpinlockLocker locker(m_config_lock);
     // Note: We must write then read to ensure completion before returning.
-    MemoryBackedHostBridge::write32_field(bus, device, function, field, value);
-    MemoryBackedHostBridge::read32_field(bus, device, function, field);
+    MemoryBackedHostBridge::write32_field_locked(bus, device, function, field, value);
+    MemoryBackedHostBridge::read32_field_locked(bus, device, function, field);
 }
 
-u8 VolumeManagementDevice::read8_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+u8 VolumeManagementDevice::read8_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
     SpinlockLocker locker(m_config_lock);
-    return MemoryBackedHostBridge::read8_field(bus, device, function, field);
+    return MemoryBackedHostBridge::read8_field_locked(bus, device, function, field);
 }
-u16 VolumeManagementDevice::read16_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+u16 VolumeManagementDevice::read16_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
     SpinlockLocker locker(m_config_lock);
-    return MemoryBackedHostBridge::read16_field(bus, device, function, field);
+    return MemoryBackedHostBridge::read16_field_locked(bus, device, function, field);
 }
-u32 VolumeManagementDevice::read32_field(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
+u32 VolumeManagementDevice::read32_field_locked(BusNumber bus, DeviceNumber device, FunctionNumber function, u32 field)
 {
     SpinlockLocker locker(m_config_lock);
-    return MemoryBackedHostBridge::read32_field(bus, device, function, field);
+    return MemoryBackedHostBridge::read32_field_locked(bus, device, function, field);
 }
 
 VolumeManagementDevice::VolumeManagementDevice(PCI::Domain const& domain, PhysicalAddress start_address)

Kernel/Bus/PCI/Controller/VolumeManagementDevice.h

@@ -20,12 +20,12 @@ public:
 private:
     VolumeManagementDevice(PCI::Domain const&, PhysicalAddress);
 
-    virtual void write8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) override;
-    virtual void write16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) override;
-    virtual void write32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) override;
-    virtual u8 read8_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-    virtual u16 read16_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
-    virtual u32 read32_field(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+    virtual void write8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u8 value) override;
+    virtual void write16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u16 value) override;
+    virtual void write32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field, u32 value) override;
+    virtual u8 read8_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+    virtual u16 read16_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
+    virtual u32 read32_field_locked(BusNumber, DeviceNumber, FunctionNumber, u32 field) override;
 
     // Note: All read and writes must be done with a spinlock because
     // Linux says that CPU might deadlock otherwise if access is not serialized.