Re: [PATCH v11 10/19] PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking

[sashiko-bot@kernel.org]

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 <leon@kernel.org>

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/20261013-fix-p2p-acs-v4-0-v11-0-72cf8c96e617@nvidia.com?part=10