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.
Thanks,
Logan
