netdev - Re: [PATCH v22 14/25] sfc: obtain decoder and region if committed by firmware

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: <7b570825-1441-408c-9d42-98d1cee11cdc@amd.com>
Date: Thu, 18 Dec 2025 12:14:00 +0000
From: Alejandro Lucero Palau <alucerop@....com>
To: Jonathan Cameron <jonathan.cameron@...wei.com>,
 alejandro.lucero-palau@....com
Cc: linux-cxl@...r.kernel.org, netdev@...r.kernel.org,
 dan.j.williams@...el.com, edward.cree@....com, davem@...emloft.net,
 kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com, dave.jiang@...el.com
Subject: Re: [PATCH v22 14/25] sfc: obtain decoder and region if committed by
 firmware


On 12/15/25 13:57, Jonathan Cameron wrote:
> On Fri, 5 Dec 2025 11:52:37 +0000
> <alejandro.lucero-palau@....com> wrote:
>
>> From: Alejandro Lucero <alucerop@....com>
>>
>> Check if device HDM is already committed during firmware/BIOS
>> initialization.
>>
>> A CXL region should exist if so after memdev allocation/initialization.
>> Get HPA from region and map it.
> I'm confused.  If this only occurs if there is a committed decoder,
> why is the exist cleanup unconditional?


Not sure I follow, but the cleanup is unconditional because it is based 
on sfc cxl initialization being successful. If not probe_data->cxl 
nothing to do.



> Looks like you add logic around this in patch 16. I think that should be
> back here for ease of reading even if for some reason this isn't broken.


I do not think so. The unwinding is different if the HDM were committed 
than if the driver did commit them (indirectly through the type2 API). 
This patch only covers the first case. The other patch adds the second 
case and the a conditional cleanup type. And in any case the cleanup 
depends on the probe_data->cxl state.


> Jonathan
>
>> Signed-off-by: Alejandro Lucero <alucerop@....com>
>> ---
>>   drivers/net/ethernet/sfc/efx_cxl.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>> index f6eda93e67e2..ad1f49e76179 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -19,6 +19,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   	struct efx_nic *efx = &probe_data->efx;
>>   	struct pci_dev *pci_dev = efx->pci_dev;
>>   	struct efx_cxl *cxl;
>> +	struct range range;
>>   	u16 dvsec;
>>   	int rc;
>>   
>> @@ -90,6 +91,26 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   		return PTR_ERR(cxl->cxlmd);
>>   	}
>>   
>> +	cxl->cxled = cxl_get_committed_decoder(cxl->cxlmd, &cxl->efx_region);
>> +	if (cxl->cxled) {
>> +		if (!cxl->efx_region) {
>> +			pci_err(pci_dev, "CXL found committed decoder without a region");
>> +			return -ENODEV;
>> +		}
>> +		rc = cxl_get_region_range(cxl->efx_region, &range);
>> +		if (rc) {
>> +			pci_err(pci_dev,
>> +				"CXL getting regions params from a committed decoder failed");
>> +			return rc;
>> +		}
>> +
>> +		cxl->ctpio_cxl = ioremap(range.start, range.end - range.start + 1);
>> +		if (!cxl->ctpio_cxl) {
>> +			pci_err(pci_dev, "CXL ioremap region (%pra) failed", &range);
>> +			return -ENOMEM;
>> +		}
>> +	}
>> +
>>   	probe_data->cxl = cxl;
>>   
>>   	return 0;
>> @@ -97,6 +118,12 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   
>>   void efx_cxl_exit(struct efx_probe_data *probe_data)
>>   {
>> +	if (!probe_data->cxl)
>> +		return;
>> +
>> +	iounmap(probe_data->cxl->ctpio_cxl);
>> +	cxl_decoder_detach(NULL, probe_data->cxl->cxled, 0, DETACH_INVALIDATE);
>> +	unregister_region(probe_data->cxl->efx_region);
>>   }
>>   
>>   MODULE_IMPORT_NS("CXL");