Hello! On Saturday 10 April 2021 16:25:24 Lukas Wunner wrote: > On Sat, Apr 10, 2021 at 02:28:45PM +0200, Pali Rohár wrote: > > I see that more PCI service drivers in their interrupt handlers are > > accessing PCI config space. > > > > E.g. in PCIe Hot Plug interrupt handler pciehp_isr handler is called > > pcie_capability_read_word() function. > > > > It is correct? Because these capability functions are protected by > > pci_lock_config() / pci_unlock_config() calls. > > Looks fine to me. That's a raw_spin_lock, which is allowed to be taken > in interrupt context. > > > And what would happen when during execution of reading or writing to > > PCIe config space (e.g. via pcie_capability_read_word()) is triggered > > PCIe HP interrupt? Would not enter interrupt handler function in > > deadlock (waiting for unlocking config space)? > > The interrupt handler would spin until the lock is released. > raw_spin_locks are not supposed to be held for a prolonged > period of time. What is "prolonged period of time"? Because for example PCIe controller on A3720 has upper limit about 1.5s when accessing config space. This is what I measured in real example. It really took PCIe HW more than 1s to return error code if it happens. > The interrupt is masked when it triggers and until it's been handled, > so the interrupt handler never runs multiple times concurrently. > See e.g. handle_level_irq() in kernel/irq/chip.c. I'm thinking if it is really safe here. On dual-core system, there may be two tasks (on each CPU) which calls pcie_capability_read_word() (or other access function). Exactly one pass pci lock. And after that two different interrupts (for each CPU) may occur which handlers would like to call pcie_capability_read_word(). raw_spin_lock_irqsave (which is called from pci_lock_config()) disable interrupts on local CPU, right? But what with second CPU? Could not be there issue? And what would happen if pcie_capability_read_word() is locked for 1.5s as described above? Does not look safe for me.
