[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <85ad74df-147b-8d27-dd39-cc9d828ada4b@amd.com>
Date: Mon, 13 Mar 2023 18:09:19 +0530
From: Gautam Dawar <gdawar@....com>
To: Jason Wang <jasowang@...hat.com>,
Gautam Dawar <gautam.dawar@....com>
Cc: linux-net-drivers@....com, Edward Cree <ecree.xilinx@...il.com>,
Martin Habets <habetsm.xilinx@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Richard Cochran <richardcochran@...il.com>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
eperezma@...hat.com, harpreet.anand@....com, tanuj.kamde@....com,
koushik.dutta@....com
Subject: Re: [PATCH net-next v2 04/14] sfc: evaluate vdpa support based on FW
capability CLIENT_CMD_VF_PROXY
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@....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@....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.
>
>> +
>> 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
>>
Powered by blists - more mailing lists