linux-kernel - Re: [PATCH v2 2/4] cxl/pci: Define a common function get_cxl

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <66fc0ff16eecf_b5d94294b7@iweiny-mobl.notmuch>
Date: Tue, 1 Oct 2024 10:06:25 -0500
From: Ira Weiny <ira.weiny@...el.com>
To: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>,
	<linux-efi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-cxl@...r.kernel.org>
CC: Ard Biesheuvel <ardb@...nel.org>, Alison Schofield
	<alison.schofield@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, "Ira
 Weiny" <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>, Yazen Ghannam
	<yazen.ghannam@....com>, Bowman Terry <terry.bowman@....com>
Subject: Re: [PATCH v2 2/4] cxl/pci: Define a common function
 get_cxl_devstate()

Smita Koralahalli wrote:
> Refactor computation of cxlds to a common function get_cxl_devstate().
> 
> The above function could then be reused in both FW-First Component and
> Protocol error reporting and handling.
> 
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
> ---
> v2:
> 	Refactor before adding trace support.
> 	get_cxl_dev() -> get_cxl_devstate().
> 	Cleaned up get_cxl_devstate().
> ---
>  drivers/cxl/pci.c | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 37164174b5fb..915102f5113f 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1021,32 +1021,38 @@ static struct pci_driver cxl_pci_driver = {
>  	},
>  };
>  
> +static struct cxl_dev_state *get_cxl_devstate(u16 segment, u8 bus,
> +					      u8 device, u8 function)
> +{
> +	unsigned int devfn = PCI_DEVFN(device, function);
> +	struct pci_dev *pdev __free(pci_dev_put) =
> +		pci_get_domain_bus_and_slot(segment, bus, devfn);

This is a good cleanup.  The previous code should not have declared pdev
NULL.  However...

> +
> +	if (!pdev)
> +		return NULL;
> +
> +	guard(device)(&pdev->dev);
> +	if (pdev->driver != &cxl_pci_driver)
> +		return NULL;
> +
> +	return pci_get_drvdata(pdev);

The device lock is now dropped before the tracing is completed.  For this
simple code I'm not keen on having the lock be taken in the helper and
released later.  It seems best to just open code this in each caller.

Ira