[PATCH v2 1/5] PCI/P2PDMA: Don't enforce ACS check for functions of same device

[Kasireddy, Vivek]

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 at google.com>
> >> Cc: Logan Gunthorpe <logang at deltatee.com>
> >> Cc: <linux-pci at vger.kernel.org>
> >> Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> >> ---
> >>  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