Messages in this thread | | | Date | Tue, 14 Mar 2023 08:38:51 +0000 | From | Martin Habets <> | Subject | Re: [PATCH net-next v2 04/14] sfc: evaluate vdpa support based on FW capability CLIENT_CMD_VF_PROXY |
| |
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@amd.com> 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@amd.com> > > > --- > > > 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.
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 > > >
| |