Re: [PATCH v1 4/4] sfc: add multipf support

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 21 Sep 2026 20:12:39 +0100
<alucerop@amd.com> wrote:

> From: Alejandro Lucero <alucerop@amd.com>
> 
> Use CXL core accelerator API for registering non-PF0 PFs to the memdev
> linked to the PF0, along with its complementary unregister.
> 
> Adapt the ioremap call per PF to be an offset based on the PF function
> index and a hardcoded per PF CXL.mem slot size.

I was wondering how you'd know what memory belonged to which one!
Simple solutions work best I suppose :)

> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/net/ethernet/sfc/efx_cxl.c | 75 ++++++++++++++++++++++++++----
>  1 file changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> index 348d7404cd7a..bed8d9c59185 100644
> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> @@ -13,6 +13,31 @@
>  #include "efx_cxl.h"
>  
>  #define EFX_CTPIO_BUFFER_SIZE	SZ_256M
> +#define EFX_CTPIO_BUFFER_PER_PF_SIZE	SZ_8M
> +
> +static int cxl_map(struct efx_probe_data *probe_data, struct efx_cxl *cxl,
> +		   u64 devfn, struct range cxl_pio_range)
> +{
> +	struct efx_nic *efx = &probe_data->efx;
> +	struct pci_dev *pci_dev = efx->pci_dev;
> +	u64 cxl_pio_pf_start;
> +
> +	cxl_pio_pf_start = cxl_pio_range.start + devfn *
> +			   EFX_CTPIO_BUFFER_PER_PF_SIZE;
Wrap as per operator precedence as easier to read.

			   cxl_pio_range.start +
			   devfn * EFX_CTPIO_BUFFER_PER_SIZE;

> +
> +	cxl->ctpio_cxl = ioremap_wc(cxl_pio_pf_start,
> +				    EFX_CTPIO_BUFFER_PER_PF_SIZE);
> +	if (!cxl->ctpio_cxl) {
> +		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
> +			&cxl_pio_range);
> +		return -ENOMEM;
> +	}
> +
> +	probe_data->cxl = cxl;
> +	probe_data->cxl_pio_initialised = true;

'map' is carry quite a lot here that isn't really about mapping anything.
Maybe think a bit more on the naming?

> +
> +	return 0;
> +}
>  
>  int efx_cxl_init(struct efx_probe_data *probe_data)
>  {
> @@ -21,8 +46,48 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>  	struct range cxl_pio_range;
>  	struct efx_cxl *cxl;
>  	u16 dvsec;
> +	u8 devfn;
>  	int rc;
>  
> +	if (efx->type->is_vf)
> +		return 0;
> +
> +	/* are we PF0? */

First things we ask seems to be Are we not PF0? 

> +	devfn = PCI_FUNC(pci_dev->devfn);
> +	if (devfn != 0) {

I'd factor this lot out as a helper to slightly improve readability.
Perhaps factor out both paths and then have an if else.


> +		struct pci_dev *pf0_pci_dev;
> +		struct cxl_memdev *cxlmd;
> +
> +		pf0_pci_dev = pci_get_slot(pci_dev->bus,
> +					   PCI_DEVFN(PCI_SLOT(pci_dev->devfn),
> +						     0));
> +		/* This should not happen! */
> +		if (!pf0_pci_dev)
> +			return 0;
> +
> +		/* Is the PF0 device configured with and using CXL? */
> +		if (!pcie_is_cxl(pf0_pci_dev))
> +			return 0;
> +
> +		cxlmd = cxl_get_pf0_memdev(&pf0_pci_dev->dev, &pci_dev->dev,
> +					   &cxl_pio_range);
> +
> +		if (IS_ERR(cxlmd))
> +			return -EPROBE_DEFER;
> +
> +		cxl = kzalloc_obj(*cxl, GFP_KERNEL);
> +		if (!cxl)
> +			return -ENOMEM;
> +
> +		cxl->cxlmd = cxlmd;
> +
> +		if (!cxl_map(probe_data, cxl, (u64)devfn, cxl_pio_range)) {
> +			kfree(cxl);
> +			return -ENOMEM;

ENOMEM for a map failure? Seems a little odd but if there is precedence
fair enough.

> +		}
> +		return 0;
> +	}
> +
>  	/* Is the device configured with and using CXL? */
>  	if (!pcie_is_cxl(pci_dev))
>  		return 0;
> @@ -80,16 +145,8 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>  		return PTR_ERR(cxl->cxlmd);
>  	}
>  
> -	cxl->ctpio_cxl = ioremap_wc(cxl_pio_range.start,
> -				    range_len(&cxl_pio_range));
> -	if (!cxl->ctpio_cxl) {
> -		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
> -			&cxl_pio_range);
> +	if (!cxl_map(probe_data, cxl, 0, cxl_pio_range))
>  		return -ENOMEM;
> -	}
> -
> -	probe_data->cxl_pio_initialised = true;
> -	probe_data->cxl = cxl;
>  
>  	return 0;
>  }