[PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach

[Andrew Rybchenko]

Summary must not be a statement. Also summary is too long.

On 6/1/23 14:42, Denis Pryazhennikov wrote:
> NIC Partitioning mode in SFN devices means multiple PFs per network port.
> When NIC Partitioning is configured, apart from the privileged adapter(s) the
> other unprivileged adapter(s) will share the same physical port.
> Determining NIC Partitioning mode is required to take necessary action(s) for
> unprivileged adapter to work seamlessly.
> NIC Partitioning is determined using heuristic approach - If the physical ports
> are shared between PFs then either NIC Partitioning or SR-IOV is in use.
> When NIC Partitioning is in use MAX MTU workaround should be applied so that
> the unprivileged functions can seamlessly configure any valid MTU.
> 
> hg-changeset: 7f0abee725a8e9c6524e773e5e5d6286a3b027a4

I'm not sure that it is appropriate here.

> 
> Signed-off-by: Sandilya Bhagi <sbhagi at solarflare.com>

I'd like to understand how is the first author of the patch.
I guess the first signed-off-by. If so, it should be in From
as well.

> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov at arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton at xilinx.com>
> ---
>   drivers/common/sfc_efx/base/ef10_nic.c | 109 +++++++++++++++++++++++++
>   drivers/common/sfc_efx/base/efx.h      |   8 ++
>   2 files changed, 117 insertions(+)
> 
> diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
> index e1709d120093..db4834a65175 100644
> --- a/drivers/common/sfc_efx/base/ef10_nic.c
> +++ b/drivers/common/sfc_efx/base/ef10_nic.c
> @@ -1044,6 +1044,89 @@ ef10_mcdi_get_pf_count(
>   	return (rc);
>   }
>   
> +static	__checkReturn	efx_rc_t
> +ef10_nic_get_physical_port_usage(
> +	__in		efx_nic_t *enp,
> +	__in_ecount(pfs_to_ports_size)	uint8_t *pfs_to_ports,
> +	__in		size_t pfs_to_ports_size,
> +	__out		efx_port_usage_t *port_usagep)
> +{
> +	efx_nic_cfg_t *encp = &(enp->en_nic_cfg);
> +	efx_port_usage_t port_usage;
> +	size_t pf;
> +	uint8_t phy_port;
> +	efx_rc_t rc;
> +
> +	/*
> +	 * The sharing of physical ports between functions are determined
> +	 * in the following way.
> +	 * 1. If VFs are enabled then the physical port is shared.
> +	 * 2. Retrieve PFs to ports assignment.
> +	 * 3. If PF 0 assignment cannot be retrieved(ACCESS_DENIED), it
> +	 *    implies this is an unprivileged function. An unprivileged
> +	 *    function indicates the physical port must be shared with
> +	 *    another privileged function.
> +	 * 4. If PF 0 assignment can be retrieved, it indicates this
> +	 *    function is privileged. Now, read all other PF's physical
> +	 *    port number assignment and check if the current PF's physical
> +	 *    port is shared with any other PF's physical port.
> +	 * NOTE: Sharing of physical ports (using heuristic approach) can
> +	 * imply either NIC Partitioning or SR-IOV is in use. This info is
> +	 * sufficient to apply the max MTU workaround (WIN-628), but should
> +	 * not be used for other purposes.

I guess you're going to use it for other purpose.

> +	 * NOTE: PF 0 is always privileged function.
> +	 */
> +
> +	if (EFX_PCI_FUNCTION_IS_VF(encp)) {
> +		port_usage = EFX_PORT_USAGE_SHARED;
> +		goto out;
> +	}
> +
> +	if (pfs_to_ports[0] ==
> +	            MC_CMD_GET_CAPABILITIES_V2_OUT_ACCESS_NOT_PERMITTED) {
> +		/*
> +		 * This is unprivileged function as it do not have sufficient
> +		 * privileges to read the value, this implies the physical port
> +		 * is shared between this function and another privileged
> +		 * function
> +		 */
> +		port_usage = EFX_PORT_USAGE_SHARED;
> +		goto out;
> +	}
> +
> +	if (encp->enc_pf >= pfs_to_ports_size) {
> +		rc = EINVAL;
> +		goto fail1;
> +	}
> +	phy_port = pfs_to_ports[encp->enc_pf];
> +
> +	/*
> +	 * This is privileged function as it is able read the value of
> +	 * PF 0. Now, check if any other function share the same physical
> +	 * port number as this function.
> +	 */
> +	for (pf = 0; pf < pfs_to_ports_size; pf++) {
> +
> +		if ((encp->enc_pf != pf) &&
> +		    (phy_port == pfs_to_ports[pf])) {
> +			/* Found match, PFs share the same physical port */
> +			port_usage = EFX_PORT_USAGE_SHARED;
> +			goto out;
> +		}
> +	}
> +
> +	port_usage = EFX_PORT_USAGE_EXCLUSIVE;
> +
> +out:
> +	*port_usagep = port_usage;
> +	return (0);
> +
> +fail1:
> +	EFSYS_PROBE1(fail1, efx_rc_t, rc);
> +
> +	return (rc);
> +}
> +
>   static	__checkReturn	efx_rc_t
>   ef10_get_datapath_caps(
>   	__in		efx_nic_t *enp)
> @@ -1307,6 +1390,32 @@ ef10_get_datapath_caps(
>   		encp->enc_tunnel_config_udp_entries_max = 0;
>   	}
>   
> +#define CAP_PFS_TO_PORTS(_n)						\
> +	(MC_CMD_GET_CAPABILITIES_V2_OUT_PFS_TO_PORTS_ASSIGNMENT_ ## _n)
> +
> +	encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
> +
> +	if (req.emr_out_length_used >= MC_CMD_GET_CAPABILITIES_V2_OUT_LEN) {
> +		/* PFs to ports assignment */
> +		uint8_t pfs_to_ports[CAP_PFS_TO_PORTS(NUM)];
> +		efx_byte_t *bytep;
> +		int i;
> +
> +		bytep = MCDI_OUT(req, efx_byte_t, CAP_PFS_TO_PORTS(OFST));
> +		for (i = 0; i < EFX_ARRAY_SIZE(pfs_to_ports); i++) {
> +			pfs_to_ports[i] = EFX_BYTE_FIELD(*bytep, EFX_BYTE_0);
> +			bytep += CAP_PFS_TO_PORTS(LEN);
> +		}

Sorry, but it looks like memcpy() byte-by-byte.

> +
> +		if (ef10_nic_get_physical_port_usage(enp,
> +		    pfs_to_ports, EFX_ARRAY_SIZE(pfs_to_ports),
> +		    &encp->enc_port_usage) != 0) {

Alignment is misleading above and hard to read. Either correct
an alignment or simply call the function before if.

> +			/* PF to port mapping lookup failed */
> +			encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
> +		}
> +	}
> +#undef  CAP_PFS_TO_PORTS
> +
>   	/*
>   	 * Check if firmware reports the VI window mode.
>   	 * Medford2 has a variable VI window size (8K, 16K or 64K).
> diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
> index 49e29dcc1c69..93bb4916bfd6 100644
> --- a/drivers/common/sfc_efx/base/efx.h
> +++ b/drivers/common/sfc_efx/base/efx.h
> @@ -311,6 +311,12 @@ efx_nic_check_pcie_link_speed(
>   	__in		uint32_t pcie_link_gen,
>   	__out		efx_pcie_link_performance_t *resultp);
>   
> +typedef enum efx_port_usage_e {
> +	EFX_PORT_USAGE_UNKNOWN = 0,
> +	EFX_PORT_USAGE_EXCLUSIVE,	/* Port only used by this PF */
> +	EFX_PORT_USAGE_SHARED,		/* Port shared with other PFs */
> +} efx_port_usage_t;
> +
>   #define	EFX_MAC_ADDR_LEN 6
>   
>   #if EFSYS_OPT_MCDI
> @@ -1680,6 +1686,8 @@ typedef struct efx_nic_cfg_s {
>   	uint32_t		enc_assigned_port;
>   	/* NIC DMA mapping type */
>   	efx_nic_dma_mapping_t	enc_dma_mapping;
> +	/* Physical ports shared by PFs */
> +	efx_port_usage_t	enc_port_usage;
>   } efx_nic_cfg_t;
>   
>   #define	EFX_PCI_VF_INVALID 0xffff