netdev - Re: [PATCH v17 20/22] sfc: create cxl region

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: <e4c00d13-9d53-4769-99cd-70b977556e7f@amd.com>
Date: Mon, 7 Jul 2025 12:37:35 +0100
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 v17 20/22] sfc: create cxl region


On 6/27/25 10:38, Jonathan Cameron wrote:
> On Tue, 24 Jun 2025 15:13:53 +0100
> <alejandro.lucero-palau@....com> wrote:
>
>> From: Alejandro Lucero <alucerop@....com>
>>
>> Use cxl api for creating a region using the endpoint decoder related to
>> a DPA range.
>>
>> Add a callback for unwinding sfc cxl initialization when the endpoint port
>> is destroyed by potential cxl_acpi or cxl_mem modules removal.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@....com>
>> ---
>>   drivers/net/ethernet/sfc/efx_cxl.c | 24 +++++++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>> index ffbf0e706330..7365effe974e 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -18,6 +18,16 @@
>>   
>>   #define EFX_CTPIO_BUFFER_SIZE	SZ_256M
>>   
>> +static void efx_release_cxl_region(void *priv_cxl)
>> +{
>> +	struct efx_probe_data *probe_data = priv_cxl;
>> +	struct efx_cxl *cxl = probe_data->cxl;
>> +
>> +	iounmap(cxl->ctpio_cxl);
>> +	cxl_put_root_decoder(cxl->cxlrd);
>> +	probe_data->cxl_pio_initialised = false;
>> +}
>> +
>>   int efx_cxl_init(struct efx_probe_data *probe_data)
>>   {
>>   	struct efx_nic *efx = &probe_data->efx;
>> @@ -116,10 +126,21 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   		goto put_root_decoder;
>>   	}
>>   
>> +	cxl->efx_region = cxl_create_region(cxl->cxlrd, &cxl->cxled, 1,
>> +					    efx_release_cxl_region,
> As per earlier comment - given when it's released, I'd register the devm callback
> out here not in cxl_create_region(). Might irritate the net maintainers though
> as it would be a devm callback registered in non CXL code, but I don't think
> that is a reason to jump through the hoops you currently have.
>
>
>> +					    &probe_data);
>> +	if (IS_ERR(cxl->efx_region)) {
>> +		pci_err(pci_dev, "CXL accel create region failed");
>> +		rc = PTR_ERR(cxl->efx_region);
>> +		goto err_region;
>> +	}
>> +
>>   	probe_data->cxl = cxl;
>>   
>>   	goto endpoint_release;
>>   
>> +err_region:
>> +	cxl_dpa_free(cxl->cxled);
>>   put_root_decoder:
>>   	cxl_put_root_decoder(cxl->cxlrd);
>>   endpoint_release:
>> @@ -129,7 +150,8 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   
>>   void efx_cxl_exit(struct efx_probe_data *probe_data)
>>   {
>> -	if (probe_data->cxl) {
>> +	if (probe_data->cxl_pio_initialised) {
> Doesn't make sense yet because it's never true yet.  I assume the code
> doesn't always fail in a way it didn't until now?


Right. My problem increasingly adding functionality. With now the sfc 
driver able to catch those potential cxl core module removals, the 
unwinding here needs to be based on this other check. But that variable 
is set by the init code in the next patch. I'll do it here in the next 
version.


Thanks!


>> +		cxl_decoder_kill_region(probe_data->cxl->cxled);
>>   		cxl_dpa_free(probe_data->cxl->cxled);
>>   		cxl_put_root_decoder(probe_data->cxl->cxlrd);
>>   	}