Re: [PATCH 22/22] sfc: support pio mapping based on cxl

[Gregory Price]

This review was generated by kreview-0811365ff2. 

This is not an automated email, the reviewer though this report
looked valid enough to consider discussion.

----
I am completely unfamiliar with this code, but the question it poses
at least seems reasonable.

~Gregory
----

On Sat, Feb 01, 2026, Alejandro Lucero wrote:

> diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
> index fcec81f862ec..2bb6d3136c7c 100644
> --- a/drivers/net/ethernet/sfc/ef10.c
> +++ b/drivers/net/ethernet/sfc/ef10.c

[ ... ]

> @@ -1263,8 +1281,25 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
>  	iounmap(efx->membase);
>  	efx->membase = membase;
>
> -	/* Set up the WC mapping if needed */
> -	if (wc_mem_map_size) {
> +	if (!wc_mem_map_size)
> +		goto skip_pio;
> +
> +	/* Set up the WC mapping */
> +
> +#ifdef CONFIG_SFC_CXL
> +	probe_data = container_of(efx, struct efx_probe_data, efx);
> +	if ((nic_data->datapath_caps3 &
> +	    (1 << MC_CMD_GET_CAPABILITIES_V7_OUT_CXL_CONFIG_ENABLE_LBN)) &&
> +	    probe_data->cxl_pio_initialised) {
> +		/* Using PIO through CXL mapping? */
> +		nic_data->pio_write_base = probe_data->cxl->ctpio_cxl +
> +					   (pio_write_vi_base * efx->vi_stride +
> +					    ER_DZ_TX_PIOBUF - uc_mem_map_size);
> +		probe_data->cxl_pio_in_use = true;
> +	} else
> +#endif
> +	{
> +		/* Using legacy PIO BAR mapping */
>  		nic_data->wc_membase = ioremap_wc(efx->membase_phys +
>  						  uc_mem_map_size,
>  						  wc_mem_map_size);
> @@ -1279,12 +1314,13 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
>  			nic_data->wc_membase +
>  			(pio_write_vi_base * efx->vi_stride + ER_DZ_TX_PIOBUF -
>  			 uc_mem_map_size);
> -
> -		rc = efx_ef10_link_piobufs(efx);
> -		if (rc)
> -			efx_ef10_free_piobufs(efx);
>  	}

The CXL path sets nic_data->pio_write_base but does not set
nic_data->pio_write_vi_base, while the legacy path does:

    nic_data->pio_write_vi_base = pio_write_vi_base;

Since nic_data is kzalloc'd, pio_write_vi_base stays at 0 in the CXL
path.  efx_ef10_link_piobufs() then uses nic_data->pio_write_vi_base
to issue MC_CMD_LINK_PIOBUF commands:

    MCDI_SET_DWORD(inbuf, LINK_PIOBUF_IN_TXQ_INSTANCE,
                   nic_data->pio_write_vi_base + index);

and also for the special-case check:

    if (tx_queue->queue == nic_data->pio_write_vi_base) {

Wouldn't this link PIO buffers to incorrect VI instances when using
CXL, since the local variable pio_write_vi_base has the correct
non-zero value but the struct field was never updated?