On Thu, Oct 01, 2026 at 12:08:07PM +0000, [email protected] 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 <[email protected]>
>
> 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/[email protected]?part=16
