linux-kernel - Re: [PATCH 4/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: <07d6242c-b6fd-f651-3c72-e3fe26eddbe5@amd.com>
Date: Thu, 23 May 2024 14:37:33 -0700
From: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
To: Dave Jiang <dave.jiang@...el.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 4/4] cxl/pci: Define a common function get_cxl_dev()

On 5/22/2024 12:42 PM, Dave Jiang wrote:
> 
> 
> On 5/22/24 8:08 AM, Smita Koralahalli wrote:
>> Refactor computation of cxlds to a common function get_cxl_dev() and reuse
>> the function in both cxl_handle_cper_event() and cxl_handle_prot_err().
> 
> I think just introduce the function where you open coded it instead of adding code and then deleting them in the same series.

Okay refactor first, then add trace support. Will change.

>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
>> ---
>>   drivers/cxl/pci.c | 52 +++++++++++++++++++++++------------------------
>>   1 file changed, 26 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index 3e3c36983686..26e65e5b68cb 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -974,32 +974,43 @@ static struct pci_driver cxl_pci_driver = {
>>   	},
>>   };
>>   
>> +static struct cxl_dev_state *get_cxl_dev(u16 segment, u8 bus, u8 device,
>> +					 u8 function)
> 
> get_cxlds() or get_cxl_device_state() would be better.

Okay will change
> 
> DJ
> 

Thanks
Smita

>> +{
>> +	struct pci_dev *pdev __free(pci_dev_put) = NULL;
>> +	struct cxl_dev_state *cxlds;
>> +	unsigned int devfn;
>> +
>> +	devfn = PCI_DEVFN(device, function);
>> +	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
>> +
>> +	if (!pdev)
>> +		return NULL;
>> +
>> +	guard(device)(&pdev->dev);
>> +	if (pdev->driver != &cxl_pci_driver)
>> +		return NULL;
>> +
>> +	cxlds = pci_get_drvdata(pdev);
>> +
>> +	return cxlds;
>> +}
>> +
>>   #define CXL_EVENT_HDR_FLAGS_REC_SEVERITY GENMASK(1, 0)
>>   static void cxl_handle_cper_event(enum cxl_event_type ev_type,
>>   				  struct cxl_cper_event_rec *rec)
>>   {
>>   	struct cper_cxl_event_devid *device_id = &rec->hdr.device_id;
>> -	struct pci_dev *pdev __free(pci_dev_put) = NULL;
>>   	enum cxl_event_log_type log_type;
>>   	struct cxl_dev_state *cxlds;
>> -	unsigned int devfn;
>>   	u32 hdr_flags;
>>   
>>   	pr_debug("CPER event %d for device %u:%u:%u.%u\n", ev_type,
>>   		 device_id->segment_num, device_id->bus_num,
>>   		 device_id->device_num, device_id->func_num);
>>   
>> -	devfn = PCI_DEVFN(device_id->device_num, device_id->func_num);
>> -	pdev = pci_get_domain_bus_and_slot(device_id->segment_num,
>> -					   device_id->bus_num, devfn);
>> -	if (!pdev)
>> -		return;
>> -
>> -	guard(device)(&pdev->dev);
>> -	if (pdev->driver != &cxl_pci_driver)
>> -		return;
>> -
>> -	cxlds = pci_get_drvdata(pdev);
>> +	cxlds = get_cxl_dev(device_id->segment_num, device_id->bus_num,
>> +			    device_id->device_num, device_id->func_num);
>>   	if (!cxlds)
>>   		return;
>>   
>> @@ -1013,21 +1024,10 @@ static void cxl_handle_cper_event(enum cxl_event_type ev_type,
>>   
>>   static void cxl_handle_prot_err(struct cxl_cper_prot_err *p_err)
>>   {
>> -	struct pci_dev *pdev __free(pci_dev_put) = NULL;
>>   	struct cxl_dev_state *cxlds;
>> -	unsigned int devfn;
>>   
>> -	devfn = PCI_DEVFN(p_err->device, p_err->function);
>> -	pdev = pci_get_domain_bus_and_slot(p_err->segment,
>> -					   p_err->bus, devfn);
>> -	if (!pdev)
>> -		return;
>> -
>> -	guard(device)(&pdev->dev);
>> -	if (pdev->driver != &cxl_pci_driver)
>> -		return;
>> -
>> -	cxlds = pci_get_drvdata(pdev);
>> +	cxlds = get_cxl_dev(p_err->segment, p_err->bus,
>> +			    p_err->device, p_err->function);
>>   	if (!cxlds)
>>   		return;
>>