Re: [PATCH v9 17/18] PCI/P2PDMA: Evaluate the ATS path for clients with ATS enabled

[Leon Romanovsky <leon@kernel.org>]

On Thu, Oct 01, 2026 at 12:07:56PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] pci/p2pdma: pcim_p2pdma_set_ats_per_mapping() silently fails for pure clients
> --
> 
> commit 39432dea495f68fb0a21d0daa1f31ba24e130b39
> Author: Leon Romanovsky <leon@kernel.org>
> 
> PCI/P2PDMA: Evaluate the ATS path for clients with ATS enabled
> 
> P2PDMA assumes every client issues Untranslated Requests. A device with
> ATS enabled may translate any address it is handed, including a bus
> address, and Translation Blocking can reject its Translated Requests on
> a route the Untranslated answer called usable. This commit takes the
> Address Type from its ATS Enable bit and avoids handing such a client
> bus addresses unless the client declared per-mapping ATS.
> 
> > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> > index 6a63901c65fb6..9c0943fdb3e36 100644
> > --- a/drivers/pci/p2pdma.c
> > +++ b/drivers/pci/p2pdma.c
> [ ... ]
> > @@ -1633,6 +1674,9 @@ pci_p2pdma_map_type_tlp(struct p2pdma_provider *provider, struct device *dev,
> >  	cache_index = map_types_idx(client);
> >  
> >  	rcu_read_lock();
> > +	/* The declaration belongs to the client, the cache to the provider. */
> > +	p2pdma = rcu_dereference(client->p2pdma);
> > +	per_mapping = p2pdma && p2pdma->ats_per_mapping;
> 
> [Severity: High]
> Does this code handle pure P2PDMA clients correctly?
> 
> Since pure clients (devices initiating P2P transfers but not providing memory)
> have no mechanism or reason to call pcim_p2pdma_init(), client->p2pdma will
> be NULL for them.
> 
> This means when they call pcim_p2pdma_set_ats_per_mapping(), it silently returns
> and drops the configuration:
> 
> drivers/pci/p2pdma.c:pcim_p2pdma_set_ats_per_mapping() {
>     ...
> 	p2p = rcu_dereference_protected(pdev->p2pdma, 1);
> 	if (!p2p)
> 		/*
> 		 * ats_per_mapping is a performance optimization,
> 		 ...
> 		 */
> 		return;
> 
> 	p2p->ats_per_mapping = true;
> }
> 
> Because client->p2pdma is NULL, per_mapping will always evaluate to false
> here in pci_p2pdma_map_type_tlp(). This leads to pci_p2pdma_client_map_type()
> incorrectly overriding the map type, which could return
> PCI_P2PDMA_MAP_NOT_SUPPORTED and break P2P transfers that should have been
> permitted.
> 
> How should pure clients register their ATS per-mapping capability if they
> do not initialize pdev->p2pdma?

I don't know there Sashiko gets these ideas, but no "pure client"
shouldn't call to pcim_p2pdma_set_ats_per_mapping().

Thanks

> 
> >  	p2pdma = rcu_dereference(pdev->p2pdma);
> >  
> >  	if (p2pdma)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261001-fix-p2p-acs-v4-0-v9-0-1a8e0f50ddd9@nvidia.com?part=17