Re: [PATCH 2/2] pci/p2pdma: Modify p2p_dma_distance to detect virtual P2P links

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Logan]

On Wed, Jun 12, 2024 at 04:27:36AM -0700, Shivasharan S wrote:
> Update the p2p_dma_distance() to determine virtual inter-switch P2P links
> existing between two switches and use this to calculate the DMA distance
> between two devices.
> 
> Signed-off-by: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
> ---
>  drivers/pci/Kconfig  |  1 +
>  drivers/pci/p2pdma.c | 18 +++++++++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index d35001589d88..3e6226ec91fd 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -174,6 +174,7 @@ config PCI_P2PDMA
>  	depends on 64BIT
>  	select GENERIC_ALLOCATOR
>  	select NEED_SG_DMA_FLAGS
> +	select PCI_SW_DISCOVERY
>  	help
>  	  Enables drivers to do PCI peer-to-peer transactions to and from
>  	  BARs that are exposed in other devices that are the part of
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 4f47a13cb500..780e649b3a1d 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -21,6 +21,8 @@
>  #include <linux/seq_buf.h>
>  #include <linux/xarray.h>
>  
> +extern bool sw_disc_check_virtual_link(struct pci_dev *a, struct pci_dev *b);

This isn't the way we declare external references.  Probably
drivers/pci/pci.h.  Would need a "pci_" or "pcie_" prefix.

>  struct pci_p2pdma {
>  	struct gen_pool *pool;
>  	bool p2pmem_published;
> @@ -576,7 +578,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
>  		int *dist, bool verbose)
>  {
>  	enum pci_p2pdma_map_type map_type = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
> -	struct pci_dev *a = provider, *b = client, *bb;
> +	struct pci_dev *a = provider, *b = client, *bb, *b_virtual_link = NULL;
>  	bool acs_redirects = false;
>  	struct pci_p2pdma *p2pdma;
>  	struct seq_buf acs_list;
> @@ -606,6 +608,17 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
>  			if (a == bb)
>  				goto check_b_path_acs;
>  
> +			// Physical Broadcom PEX switches can be provisioned into
> +			// multiple virtual switches.
> +			// if both upstream bridges belong to the same physical
> +			// switch, and the switch supports P2P,
> +			// p2p_dma_distance() should take into account of such
> +			// scenarios.

Copy the comment style (/* */) of the rest of the file.  Rewrap to fit
in 80 columns like the rest of the file.  Capitalize sentences.  Add
blank line between paragraphs.

> +			if (sw_disc_check_virtual_link(a, bb)) {
> +				b_virtual_link = bb;
> +				goto check_b_path_acs;
> +			}
> +
>  			bb = pci_upstream_bridge(bb);
>  			dist_b++;
>  		}
> @@ -629,6 +642,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
>  			acs_cnt++;
>  		}
>  
> +		if (b_virtual_link && bb == b_virtual_link)
> +			break;
> +
>  		bb = pci_upstream_bridge(bb);
>  	}
>  
> -- 
> 2.43.0
>