Re: [PATCH net-next v2 04/14] sfc: evaluate vdpa support based on FW capability CLIENT_CMD_VF_PROXY

From: Gautam Dawar
Date: Fri Mar 17 2023 - 07:19:26 EST



On 3/14/23 14:08, Martin Habets wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Mon, Mar 13, 2023 at 06:09:19PM +0530, Gautam Dawar wrote:
On 3/10/23 10:34, Jason Wang wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


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?
This isn't actually fixing any issue. This a minor code refactoring that
wraps-up updating of the TSO capabilities in a separate function for better
readability.
There definity was an issue here: the vDPA code now needs access to the NIC
capabilities. For this is should use the efx_ef100_init_datapath_caps below,
but that was also doing this netdev specific stuff.
The solution is to split up efx_ef100_init_datapath_caps into a generic API
that vDPA can use, and this netdev specific API which should not be used by vDPA.

Gautam, you could explain this API split in the description.

Sure, will update the commit description with this information.

Thanks


Martin

+
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.
Sure, will fix.

Thanks

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