Re: [PATCH v29 4/5] sfc: obtain and map cxl range using devm_cxl_probe_mem

[Alejandro Lucero Palau <alucerop@amd.com>]

On 6/26/26 04:52, Richard Cheng wrote:
> On Mon, Jun 22, 2026 at 01:40:09PM +0800, alejandro.lucero-palau@amd.com wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Use core API for safely obtain the CXL range linked to an HDM committed
>> by the BIOS. Map such a range for being used as the ctpio buffer.
>>
>> A potential user space action through sysfs unbinding or core cxl
>> modules remove will trigger sfc driver device detachment, with that case
>> not racing with this mapping as this is done during driver probe and
>> therefore protected with device lock against those user space actions.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
>> ---
>>   drivers/net/ethernet/sfc/efx.c     |  2 ++
>>   drivers/net/ethernet/sfc/efx_cxl.c | 23 +++++++++++++++++++++++
>>   drivers/net/ethernet/sfc/efx_cxl.h |  3 +++
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
>> index 61cbb6cfc360..3806cd3dd7f4 100644
>> --- a/drivers/net/ethernet/sfc/efx.c
>> +++ b/drivers/net/ethernet/sfc/efx.c
>> @@ -984,6 +984,7 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
>>   	efx_fini_io(efx);
>>   
>>   	probe_data = container_of(efx, struct efx_probe_data, efx);
>> +	efx_cxl_exit(probe_data);
>>   
>>   	pci_dbg(efx->pci_dev, "shutdown successful\n");
>>   
>> @@ -1242,6 +1243,7 @@ static int efx_pci_probe(struct pci_dev *pci_dev,
>>   	return 0;
>>   
>>    fail3:
>> +	efx_cxl_exit(probe_data);
>>   	efx_fini_io(efx);
>>    fail2:
>>   	efx_fini_struct(efx);
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>> index 18b535b3ea40..3e7c950f83e9 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -18,6 +18,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 range cxl_pio_range;
>>   	struct efx_cxl *cxl;
>>   	u16 dvsec;
>>   	int rc;
>> @@ -73,9 +74,31 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   		return -ENODEV;
>>   	}
>>   
>> +	cxl->cxlmd = devm_cxl_probe_mem(&cxl->cxlds, &cxl_pio_range);
>> +	if (IS_ERR(cxl->cxlmd)) {
>> +		pci_err(pci_dev, "CXL accel memdev creation failed\n");
>> +		return PTR_ERR(cxl->cxlmd);
>> +	}
>> +
>> +	cxl->ctpio_cxl = ioremap_wc(cxl_pio_range.start,
>> +				    range_len(&cxl_pio_range));
> Hi Alejandro,
>
> A small question here,
> Is it possible that the FW would commit a region bigger than the range ?


Hi Richard,


Not really. We are using the minimum size for CXL mem, 256MB, and it is 
not configurable. If CXL is enabled by the sfc device firmware, this is 
the only possibility.

It could be a good sanity check though, but I prefer to keep v29 as it 
is now ... Dan Williams is happy enough with it, so I expect Dave to 
merge it soon ...


Maybe as a follow up path.


Thank you,

Alejandro


> The committed CXL region length is never validated against the PIO window size.
> The legacy patch sizes wc_mem_map_size to cover the VI-strided PIO offset, but
> here we ioremap whatever the BIOS comitted and assume it's EFX_CTPIO_BUFFER_SIZE.
>
> Maybe adding
> """
> if (range_len(&cxl_pio_range) < EFX_CTPIO_BUFFER_SIZE)
>      return -EINVAL;
> """
> Would be worthy ?
>
> Let me know what you think.
>
> Best regards,
> Richard Cheng
>
>> +	if (!cxl->ctpio_cxl) {
>> +		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
>> +			&cxl_pio_range);
>> +		return -ENOMEM;
>> +	}
>> +
>>   	probe_data->cxl = cxl;
>>   
>>   	return 0;
>>   }
>>   
>> +void efx_cxl_exit(struct efx_probe_data *probe_data)
>> +{
>> +	if (!probe_data->cxl)
>> +		return;
>> +
>> +	iounmap(probe_data->cxl->ctpio_cxl);
>> +}
>> +
>>   MODULE_IMPORT_NS("CXL");
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h
>> index 04e46278464d..3e2705cb063f 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.h
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.h
>> @@ -20,10 +20,13 @@ struct efx_probe_data;
>>   struct efx_cxl {
>>   	struct cxl_dev_state cxlds;
>>   	struct cxl_memdev *cxlmd;
>> +	void __iomem *ctpio_cxl;
>>   };
>>   
>>   int efx_cxl_init(struct efx_probe_data *probe_data);
>> +void efx_cxl_exit(struct efx_probe_data *probe_data);
>>   #else
>>   static inline int efx_cxl_init(struct efx_probe_data *probe_data) { return 0; }
>> +static inline void efx_cxl_exit(struct efx_probe_data *probe_data) {}
>>   #endif
>>   #endif
>> -- 
>> 2.34.1
>>
>>