linux-kernel - Re: [PATCH 2/2 v2] PCI/P2PDMA: Modify p2p_dma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240924160827.000049dd@Huawei.com>
Date: Tue, 24 Sep 2024 16:08:27 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Shivasharan S <shivasharan.srikanteshwara@...adcom.com>
CC: <linux-pci@...r.kernel.org>, <bhelgaas@...gle.com>,
	<manivannan.sadhasivam@...aro.org>, <logang@...tatee.com>,
	<linux-kernel@...r.kernel.org>, <sumanesh.samanta@...adcom.com>,
	<sathya.prakash@...adcom.com>, <sjeaugey@...dia.com>
Subject: Re: [PATCH 2/2 v2] PCI/P2PDMA: Modify p2p_dma_distance to detect
 P2P links

On Thu, 19 Sep 2024 01:13:44 -0700
Shivasharan S <shivasharan.srikanteshwara@...adcom.com> wrote:

> Update the p2p_dma_distance() to determine 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@...adcom.com>
> ---
>  drivers/pci/p2pdma.c       | 17 ++++++++++++++++-
>  drivers/pci/pcie/portdrv.c | 34 ++++++++++++++++++++++++++++++++++
>  drivers/pci/pcie/portdrv.h |  2 ++
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 4f47a13cb500..eed3b69e7293 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 pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b);

That's nasty.  Include the header so you get a clean stub if
this support is not built in etc.

> +

> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index c940b4b242fd..2fe1598fc684 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -104,6 +104,40 @@ static bool pcie_port_is_p2p_supported(struct pci_dev *dev)
>  	return false;
>  }
>  
> +/**
> + * pcie_port_is_p2p_link_available: Determine if a P2P link is available
> + * between the two upstream bridges. The serial number of the two devices
> + * will be compared and if they are same then it is considered that the P2P
> + * link is available.
> + *
> + * Return value: true if inter switch P2P is available, return false otherwise.
> + */
> +bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b)
> +{
> +	u64 dsn_a, dsn_b;
> +
> +	/*
> +	 * Check if the devices support Inter switch P2P.
> +	 */

Single line comment syntax fine here.  However it's kind
of obvious, so I'd just drop the comment.


> +	if (!pcie_port_is_p2p_supported(a) ||
> +	    !pcie_port_is_p2p_supported(b))

Don't wrap this. I think it's under 80 chars anyway.

> +		return false;
> +
> +	dsn_a = pci_get_dsn(a);
> +	if (!dsn_a)
> +		return false;
If we assume that dsn is the only right way to detect this
(I'm fine with that for now) then we know the supported tests
above would only pass if this is true. Hence

return pci_get_dsn(a) == pci_get_dsn(b);

should be fine.

> +
> +	dsn_b = pci_get_dsn(b);
> +	if (!dsn_b)
> +		return false;
> +
> +	if (dsn_a == dsn_b)
> +		return true;

	return dsn_a == dsn_b;

> +
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(pcie_port_is_p2p_link_available);
> +
>  /*
>   * Traverse list of all PCI bridges and find devices that support Inter switch P2P
>   * and have the same serial number to create report the BDF over sysfs.