Re: [PATCH net-next v2 04/14] sfc: evaluate vdpa support based on FW capability CLIENT_CMD_VF_PROXY
From: Jason Wang
Date: Fri Mar 10 2023 - 00:05:26 EST
On Tue, Mar 7, 2023 at 7:37 PM Gautam Dawar <gautam.dawar@xxxxxxx> wrote:
>
> Add and update vdpa_supported field to struct efx_nic to true if
> running Firmware supports CLIENT_CMD_VF_PROXY capability. This is
> required to ensure DMA isolation between MCDI command buffer and guest
> buffers.
>
> Signed-off-by: Gautam Dawar <gautam.dawar@xxxxxxx>
> ---
> drivers/net/ethernet/sfc/ef100_netdev.c | 26 +++++++++++++++---
> drivers/net/ethernet/sfc/ef100_nic.c | 35 +++++++++----------------
> drivers/net/ethernet/sfc/ef100_nic.h | 6 +++--
> drivers/net/ethernet/sfc/ef100_vdpa.h | 5 ++--
> 4 files changed, 41 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> index d916877b5a9a..5d93e870d9b7 100644
> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> @@ -355,6 +355,28 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
> efx->state = STATE_PROBED;
> }
>
> +static void efx_ef100_update_tso_features(struct efx_nic *efx)
> +{
> + struct ef100_nic_data *nic_data = efx->nic_data;
> + struct net_device *net_dev = efx->net_dev;
> + netdev_features_t tso;
> +
> + if (!efx_ef100_has_cap(nic_data->datapath_caps2, TX_TSO_V3))
> + return;
> +
> + tso = NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_PARTIAL |
> + NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_UDP_TUNNEL_CSUM |
> + NETIF_F_GSO_GRE | NETIF_F_GSO_GRE_CSUM;
> +
> + net_dev->features |= tso;
> + net_dev->hw_features |= tso;
> + net_dev->hw_enc_features |= tso;
> + /* EF100 HW can only offload outer checksums if they are UDP,
> + * so for GRE_CSUM we have to use GSO_PARTIAL.
> + */
> + net_dev->gso_partial_features |= NETIF_F_GSO_GRE_CSUM;
> +}
I don't see a direct relationship between vDPA and the TSO capability.
Is this an independent fix?
> +
> int ef100_probe_netdev(struct efx_probe_data *probe_data)
> {
> struct efx_nic *efx = &probe_data->efx;
> @@ -387,9 +409,7 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
> ESE_EF100_DP_GZ_TSO_MAX_HDR_NUM_SEGS_DEFAULT);
> efx->mdio.dev = net_dev;
>
> - rc = efx_ef100_init_datapath_caps(efx);
> - if (rc < 0)
> - goto fail;
> + efx_ef100_update_tso_features(efx);
>
> rc = ef100_phy_probe(efx);
> if (rc)
> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
> index 8cbe5e0f4bdf..ef6e295efcf7 100644
> --- a/drivers/net/ethernet/sfc/ef100_nic.c
> +++ b/drivers/net/ethernet/sfc/ef100_nic.c
> @@ -161,7 +161,7 @@ int ef100_get_mac_address(struct efx_nic *efx, u8 *mac_address,
> return 0;
> }
>
> -int efx_ef100_init_datapath_caps(struct efx_nic *efx)
> +static int efx_ef100_init_datapath_caps(struct efx_nic *efx)
> {
> MCDI_DECLARE_BUF(outbuf, MC_CMD_GET_CAPABILITIES_V7_OUT_LEN);
> struct ef100_nic_data *nic_data = efx->nic_data;
> @@ -197,25 +197,15 @@ int efx_ef100_init_datapath_caps(struct efx_nic *efx)
> if (rc)
> return rc;
>
> - if (efx_ef100_has_cap(nic_data->datapath_caps2, TX_TSO_V3)) {
> - struct net_device *net_dev = efx->net_dev;
> - netdev_features_t tso = NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_PARTIAL |
> - NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_UDP_TUNNEL_CSUM |
> - NETIF_F_GSO_GRE | NETIF_F_GSO_GRE_CSUM;
> -
> - net_dev->features |= tso;
> - net_dev->hw_features |= tso;
> - net_dev->hw_enc_features |= tso;
> - /* EF100 HW can only offload outer checksums if they are UDP,
> - * so for GRE_CSUM we have to use GSO_PARTIAL.
> - */
> - net_dev->gso_partial_features |= NETIF_F_GSO_GRE_CSUM;
> - }
> efx->num_mac_stats = MCDI_WORD(outbuf,
> GET_CAPABILITIES_V4_OUT_MAC_STATS_NUM_STATS);
> netif_dbg(efx, probe, efx->net_dev,
> "firmware reports num_mac_stats = %u\n",
> efx->num_mac_stats);
> +
> + nic_data->vdpa_supported = efx_ef100_has_cap(nic_data->datapath_caps3,
> + CLIENT_CMD_VF_PROXY) &&
> + efx->type->is_vf;
> return 0;
> }
>
> @@ -806,13 +796,6 @@ static char *bar_config_name[] = {
> [EF100_BAR_CONFIG_VDPA] = "vDPA",
> };
>
> -#ifdef CONFIG_SFC_VDPA
> -static bool efx_vdpa_supported(struct efx_nic *efx)
> -{
> - return efx->type->is_vf;
> -}
> -#endif
> -
> int efx_ef100_set_bar_config(struct efx_nic *efx,
> enum ef100_bar_config new_config)
> {
> @@ -828,7 +811,7 @@ int efx_ef100_set_bar_config(struct efx_nic *efx,
>
> #ifdef CONFIG_SFC_VDPA
> /* Current EF100 hardware supports vDPA on VFs only */
> - if (new_config == EF100_BAR_CONFIG_VDPA && !efx_vdpa_supported(efx)) {
> + if (new_config == EF100_BAR_CONFIG_VDPA && !nic_data->vdpa_supported) {
> pci_err(efx->pci_dev, "vdpa over PF not supported : %s",
> efx->name);
> return -EOPNOTSUPP;
> @@ -1208,6 +1191,12 @@ static int ef100_probe_main(struct efx_nic *efx)
> goto fail;
> }
>
> + rc = efx_ef100_init_datapath_caps(efx);
> + if (rc) {
> + pci_info(efx->pci_dev, "Unable to initialize datapath caps\n");
> + goto fail;
> + }
> +
> return 0;
> fail:
> return rc;
> diff --git a/drivers/net/ethernet/sfc/ef100_nic.h b/drivers/net/ethernet/sfc/ef100_nic.h
> index 4562982f2965..117a73d0795c 100644
> --- a/drivers/net/ethernet/sfc/ef100_nic.h
> +++ b/drivers/net/ethernet/sfc/ef100_nic.h
> @@ -76,6 +76,9 @@ struct ef100_nic_data {
> u32 datapath_caps3;
> unsigned int pf_index;
> u16 warm_boot_count;
> +#ifdef CONFIG_SFC_VDPA
> + bool vdpa_supported; /* true if vdpa is supported on this PCIe FN */
> +#endif
> u8 port_id[ETH_ALEN];
> DECLARE_BITMAP(evq_phases, EFX_MAX_CHANNELS);
> enum ef100_bar_config bar_config;
> @@ -95,9 +98,8 @@ struct ef100_nic_data {
> };
>
> #define efx_ef100_has_cap(caps, flag) \
> - (!!((caps) & BIT_ULL(MC_CMD_GET_CAPABILITIES_V4_OUT_ ## flag ## _LBN)))
> + (!!((caps) & BIT_ULL(MC_CMD_GET_CAPABILITIES_V7_OUT_ ## flag ## _LBN)))
>
> -int efx_ef100_init_datapath_caps(struct efx_nic *efx);
> int ef100_phy_probe(struct efx_nic *efx);
> int ef100_filter_table_probe(struct efx_nic *efx);
>
> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h b/drivers/net/ethernet/sfc/ef100_vdpa.h
> index f6564448d0c7..90062fd8a25d 100644
> --- a/drivers/net/ethernet/sfc/ef100_vdpa.h
> +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h
> @@ -1,7 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> -/* Driver for Xilinx network controllers and boards
> - * Copyright (C) 2020-2022, Xilinx, Inc.
> - * Copyright (C) 2022, Advanced Micro Devices, Inc.
> +/* Driver for AMD network controllers and boards
> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
Let's fix this in the patch that introduces this.
Thanks
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms of the GNU General Public License version 2 as published
> --
> 2.30.1
>