Re: [PATCH v9 16/18] PCI/P2PDMA: Let a client declare that it selects ATS per mapping

[Leon Romanovsky <leon@kernel.org>]

On Thu, Oct 01, 2026 at 12:08:07PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] pci/p2pdma: pure clients cannot set ats_per_mapping because pdev->p2pdma is NULL
> --
> 
> commit f332086fc4a4273d099c81fafb33298021d47d1a
> Author: Leon Romanovsky <leon@kernel.org>
> 
> PCI/P2PDMA: Let a client declare that it selects ATS per mapping
> 
> The PCIe ATS Enable bit covers the whole device, so P2PDMA cannot tell
> from it whether a client will translate a given address. Most devices
> translate any address once ATS is enabled, but some choose ATS per DMA
> mapping and can still use bus addresses for the rest.
> 
> Add pcim_p2pdma_set_ats_per_mapping() so the driver of such a device can
> declare that before P2PDMA starts treating clients with ATS enabled as
> translating everything.
> 
> > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> > index cf190bd572d4a..6a63901c65fb6 100644
> > --- a/drivers/pci/p2pdma.c
> > +++ b/drivers/pci/p2pdma.c
> 
> [ ... ]
> 
> > @@ -1562,6 +1563,32 @@ void pci_p2pmem_publish(struct pci_dev *pdev, bool publish)
> >  }
> >  EXPORT_SYMBOL_GPL(pci_p2pmem_publish);
> >  
> > +/**
> > + * pcim_p2pdma_set_ats_per_mapping - Declare per-mapping ATS for a client
> > + * @pdev: PCI device that initiates peer-to-peer DMA
> > + *
> > + * Declare that @pdev issues Translated Requests only for the DMA mappings its
> > + * driver sets up to use ATS, rather than for any address once ATS is enabled.
> > + * P2PDMA then routes this client's Requests with the Address Type each query
> > + * asks about, rather than the one its ATS Enable bit implies.
> > + */
> > +void pcim_p2pdma_set_ats_per_mapping(struct pci_dev *pdev)
> > +{
> > +	struct pci_p2pdma *p2p;
> > +
> > +	p2p = rcu_dereference_protected(pdev->p2pdma, 1);
> > +	if (!p2p)
> > +		/*
> > +		 * ats_per_mapping is a performance optimization,
> > +		 * if pcim_p2pdma_init() didn't set pdev->p2pdma pointer
> > +		 * for some reason, let's simply use ATS global settings.
> > +		 */
> > +		return;
> 
> [Severity: Medium]
> Will this silently fail for pure P2PDMA clients?
> 
> It looks like pdev->p2pdma is only allocated when a device registers as a
> P2P memory provider via pcim_p2pdma_init():

Yes, this is exactly for them and no one else.

Thanks

> 
> drivers/pci/p2pdma.c:pcim_p2pdma_init() {
> 	...
> 	p2p = devm_kzalloc(&pdev->dev, sizeof(*p2p), GFP_KERNEL);
> 	...
> }
> 
> If a driver for a pure P2PDMA client (e.g., an NVMe initiator without CMB)
> calls pcim_p2pdma_set_ats_per_mapping() to declare its ATS behavior, won't
> pdev->p2pdma be NULL since it never called pcim_p2pdma_init()?

Only RDMA NICs support that per-mapping thing.

Thanks

> 
> If so, wouldn't this cause the client to silently fall back to suboptimal
> global ATS routing, as it can never successfully set the ats_per_mapping flag?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261001-fix-p2p-acs-v4-0-v9-0-1a8e0f50ddd9@nvidia.com?part=16