Hi Logan,
> Subject: Re: [PATCH v2 1/5] PCI/P2PDMA: Don't enforce ACS check for
> functions of same device
>
>
>
> On 2024-10-22 09:16, Bjorn Helgaas wrote:
> > On Sun, Oct 20, 2024 at 10:21:29PM -0700, Vivek Kasireddy wrote:
> >> Functions of the same PCI device (such as a PF and a VF) share the
> >> same bus and have a common root port and typically, the PF provisions
> >> resources for the VF. Therefore, they can be considered compatible
> >> as far as P2P access is considered.
> >>
> >> Currently, although the distance (2) is correctly calculated for
> >> functions of the same device, an ACS check failure prevents P2P DMA
> >> access between them. Therefore, introduce a small function named
> >> pci_devs_are_p2pdma_compatible() to determine if the provider and
> >> client belong to the same device and facilitate P2P DMA between
> >> them by not enforcing the ACS check.
> >>
> >> v2:
> >> - Relax the enforcment of ACS check only for Intel GPU functions
> >> as they are P2PDMA compatible given the way the PF provisions
> >> the resources among multiple VFs.
> >
> > I don't want version history in the commit log. If the content is
> > useful, just incorporate it here directly (without the version info),
> > and put the version-to-version changelog below the "---".
> >
> >> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >> Cc: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> >> Cc: <linux-pci@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> >> ---
> >> drivers/pci/p2pdma.c | 17 +++++++++++++++--
> >> 1 file changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> >> index 4f47a13cb500..a230e661f939 100644
> >> --- a/drivers/pci/p2pdma.c
> >> +++ b/drivers/pci/p2pdma.c
> >> @@ -535,6 +535,17 @@ static unsigned long map_types_idx(struct
> pci_dev *client)
> >> return (pci_domain_nr(client->bus) << 16) | pci_dev_id(client);
> >> }
> >>
> >> +static bool pci_devs_are_p2pdma_compatible(struct pci_dev *provider,
> >> + struct pci_dev *client)
> >> +{
> >> + if (provider->vendor == PCI_VENDOR_ID_INTEL) {
> >> + if (pci_is_vga(provider) && pci_is_vga(client))
> >> + return pci_physfn(provider) == pci_physfn(client);
> >> + }
>
> I'd echo many of Bjorn's concerns. In addition, I think the name of the
> pci_devs_are_p2pdma_compatible() isn't quite right. Specifically this is
> dealing with PCI functions within a single device that are known to
> allow P2P traffic. So I think the name should probably reflect that.
Would pci_devfns_support_p2pdma() be a more appropriate name?
Thanks,
Vivek
>
> Thanks,
>
> Logan
