Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] pci/p2pdma: deadlock in calc_map_type_and_dist() due to non-IRQ-safe xa_store() --
commit f019da119f00a99a75cbfe2deec0db131a89232d Author: Leon Romanovsky <[email protected]> PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking ACS Translation Blocking rejects upstream Memory Requests with a Translated address before any peer-routing controls apply, per PCIe r7.0 sec 6.12.1.1. P2PDMA ignores that bit and can report a route whose ports reject the Requester's address type. Add PCI_ACS_P2PDMA_BLOCKED and scan the ports traversed by each route. A direct route passes only ports below the divergence; a host-bridge route also passes the common port and its ancestors. Reject the affected class without falling back to another route, and preserve blocking precedence over internal-route restrictions. > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index 785e4668a2af5..fc4dea28c8606 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c [ ... ] > @@ -1162,6 +1252,11 @@ calc_map_type_and_dist(struct pci_dev *provider, > struct pci_dev *client, > } > } > > + path.tb_on_path = pci_p2pdma_path_blocks_translation(client, a); > + if (b_child) > + path.tb_above_divergence = > + pci_p2pdma_path_blocks_translation(b_child, NULL); > + > /* > * The walk and the config reads above serve every class; only the > * decision below depends on the kind of TLP being routed. [Severity: High] Does xa_store() use an IRQ-safe lock here? The calc_map_type_and_dist() function caches routing results by calling xa_store() later in this function: xa_store(&p2pdma->map_types, map_types_idx(client), xa_mk_value(pci_p2pdma_map_types_pack(map_type)), GFP_ATOMIC); xa_store() implicitly acquires xa->xa_lock using a plain spin_lock(), which does not disable hardware or software interrupts. Since calc_map_type_and_dist() is reachable from process context (e.g., during target configuration via pci_p2pmem_find_many()), what happens if a P2PDMA IO mapping request is processed in softirq or hardirq (e.g., dma_map_sg -> pci_p2pdma_map_type_tlp during an NVMe-oF RDMA completion) on the same CPU and experiences a cache miss? It seems it will interrupt the process context, re-enter calc_map_type_and_dist(), and attempt to acquire the same xa->xa_lock via xa_store(). Could this result in a hard deadlock since the lock is already held by the interrupted thread on the same CPU? Additionally, the XArray appears to be initialized without XA_FLAGS_LOCK_IRQ in pcim_p2pdma_init(): xa_init(&p2p->map_types); To safely update the XArray from contexts that include interrupts, should the driver acquire the lock with IRQs disabled (e.g., using xa_lock_irqsave(&p2pdma->map_types, flags); __xa_store(...); xa_unlock_irqrestore(...))? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
